Skip to content

Commit

Permalink
Fix messages leaking via suspended messages
Browse files Browse the repository at this point in the history
It isn't clear what StoreReporter's purpose is.  Is it simply a store of
all messages, or is it a store of "real" messages, i.e. messages that
aren't suppressed with `@nowarn` or -Wconf (i.e. configurable warnings)?
I believe StoreReporter is often used as a way to get programmatic
access to the real messages.

Typer, with its TyperState, uses StoreReporter as a more general buffer
while making typing attempts, such as when trying to apply an implicit
conversion.  But with configurable warnings, we don't know if a warning
is real or not until we've typed all the `@nowarn` annotation in the
code, which is why we suspend the messages, on Run.

So we add an override for TyperState, so that StoreReporter is used as
simply a buffer.  When it then unbuffers its messages in flush to the
parent context's reporter, it will run through the regular
"issueIfNotSuppressed" code (assuming it's not another store reporter).
  • Loading branch information
dwijnand committed Nov 8, 2021
1 parent 483bf2c commit 31d6b99
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 37 deletions.
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/TyperState.scala
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class TyperState() {
this

/** A fresh typer state with the same constraint as this one. */
def fresh(reporter: Reporter = StoreReporter(this.reporter),
def fresh(reporter: Reporter = StoreReporter(this.reporter, fromTyperState = true),
committable: Boolean = this.isCommittable): TyperState =
util.Stats.record("TyperState.fresh")
TyperState().init(this, this.constraint)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import core.Contexts.Context
import Diagnostic._

/** A re-usable Reporter used in Contexts#test */
class ExploringReporter extends StoreReporter(null):
class ExploringReporter extends StoreReporter(null, fromTyperState = false):
infos = new mutable.ListBuffer[Diagnostic]

override def hasUnreportedErrors: Boolean =
Expand Down
68 changes: 35 additions & 33 deletions compiler/src/dotty/tools/dotc/reporting/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -137,42 +137,44 @@ abstract class Reporter extends interfaces.ReporterResult {

var unreportedWarnings: Map[String, Int] = Map.empty

/** Issue the diagnostic, ignoring `-Wconf` and `@nowarn` configurations,
* but still honouring `-nowarn`, `-Werror`, and conditional warnings. */
def issueUnconfigured(dia: Diagnostic)(using Context): Unit = dia match
case w: Warning if ctx.settings.silentWarnings.value =>
case w: ConditionalWarning if w.isSummarizedConditional =>
val key = w.enablingOption.name
val count = unreportedWarnings.getOrElse(key, 0)
unreportedWarnings = unreportedWarnings.updated(key, count + 1)
case _ =>
// conditional warnings that are not enabled are not fatal
val d = dia match
case w: Warning if ctx.settings.XfatalWarnings.value => w.toError
case _ => dia
if !isHidden(d) then // avoid isHidden test for summarized warnings so that message is not forced
withMode(Mode.Printing)(doReport(d))
d match {
case _: Warning => _warningCount += 1
case e: Error =>
errors = e :: errors
_errorCount += 1
if ctx.typerState.isGlobalCommittable then
ctx.base.errorsToBeReported = true
case _: Info => // nothing to do here
// match error if d is something else
}
end issueUnconfigured

def issueIfNotSuppressed(dia: Diagnostic)(using Context): Unit =
def go() =
import Action._

val toReport = dia match
case w: Warning =>
def fatal(w: Warning) = if ctx.settings.XfatalWarnings.value && !w.isSummarizedConditional then Some(w.toError) else Some(w)
if ctx.settings.silentWarnings.value then None
else WConf.parsed.action(dia) match
case Silent => None
case Info => Some(w.toInfo)
case Warning => fatal(w)
case Verbose => fatal(w).tap(_.foreach(_.setVerbose()))
case Error => Some(w.toError)
case _ => Some(dia)

toReport foreach {
case cw: ConditionalWarning if cw.isSummarizedConditional =>
val key = cw.enablingOption.name
unreportedWarnings =
unreportedWarnings.updated(key, unreportedWarnings.getOrElse(key, 0) + 1)
case d if !isHidden(d) =>
withMode(Mode.Printing)(doReport(d))
d match {
case _: Warning => _warningCount += 1
case e: Error =>
errors = e :: errors
_errorCount += 1
if ctx.typerState.isGlobalCommittable then
ctx.base.errorsToBeReported = true
case _: Info => // nothing to do here
// match error if d is something else
}
case _ => // hidden
}
end go
dia match
case w: Warning => WConf.parsed.action(w) match
case Error => issueUnconfigured(w.toError)
case Warning => issueUnconfigured(w)
case Verbose => issueUnconfigured(w.setVerbose())
case Info => issueUnconfigured(w.toInfo)
case Silent =>
case _ => issueUnconfigured(dia)

// `ctx.run` can be null in test, also in the repl when parsing the first line. The parser runs early, the Run is
// only created in ReplDriver.compile when a line is submitted. This means that `@nowarn` doesnt work on parser
Expand Down
9 changes: 8 additions & 1 deletion compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import Diagnostic._
* - The reporter is not flushed and the message containers capture a
* `Context` (about 4MB)
*/
class StoreReporter(outer: Reporter = Reporter.NoReporter) extends Reporter {
class StoreReporter(outer: Reporter = Reporter.NoReporter, fromTyperState: Boolean = false) extends Reporter {

protected var infos: mutable.ListBuffer[Diagnostic] = null

Expand All @@ -40,4 +40,11 @@ class StoreReporter(outer: Reporter = Reporter.NoReporter) extends Reporter {
override def pendingMessages(using Context): List[Diagnostic] = if (infos != null) infos.toList else Nil

override def errorsReported: Boolean = hasErrors || (outer != null && outer.errorsReported)

// If this is a TyperState buffering reporter then buffer the messages,
// so that then only when the messages are unbuffered (when the reporter if flushed)
// do they go through -Wconf, and possibly then buffered on the Run as a suspended message
override def report(dia: Diagnostic)(using Context): Unit =
if fromTyperState then issueUnconfigured(dia)
else super.report(dia)
}
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/reporting/TestReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import collection.mutable
import Diagnostic._

/** A re-usable Reporter used in Contexts#test */
class TestingReporter extends StoreReporter(null):
class TestingReporter extends StoreReporter(null, fromTyperState = false):
infos = new mutable.ListBuffer[Diagnostic]
override def hasUnreportedErrors: Boolean = infos.exists(_.isInstanceOf[Error])
def reset(): Unit = infos.clear()

0 comments on commit 31d6b99

Please sign in to comment.