Merge pull request #1696 from felixmulder/topic/assert-message-laziness

Make sure messages are lazily evaluated until `report` in `Reporter`

Author: felixmulder

src/dotty/tools/dotc/parsing/Parsers.scala

@@ -99,7 +99,7 @@ object Parsers {
     /** Issue an error at given offset if beyond last error offset
       *  and update lastErrorOffset.
       */
-    def syntaxError(msg: Message, offset: Int = in.offset): Unit =
+    def syntaxError(msg: => Message, offset: Int = in.offset): Unit =
       if (offset > lastErrorOffset) {
         syntaxError(msg, Position(offset))
         lastErrorOffset = in.offset
@@ -108,7 +108,7 @@ object Parsers {
     /** Unconditionally issue an error at given position, without
       *  updating lastErrorOffset.
       */
-    def syntaxError(msg: Message, pos: Position): Unit =
+    def syntaxError(msg: => Message, pos: Position): Unit =
       ctx.error(msg, source atPos pos)
 
   }
@@ -215,23 +215,23 @@ object Parsers {
       }
     }
 
-    def warning(msg: Message, sourcePos: SourcePosition) =
+    def warning(msg: => Message, sourcePos: SourcePosition) =
       ctx.warning(msg, sourcePos)
 
-    def warning(msg: Message, offset: Int = in.offset) =
+    def warning(msg: => Message, offset: Int = in.offset) =
       ctx.warning(msg, source atPos Position(offset))
 
-    def deprecationWarning(msg: Message, offset: Int = in.offset) =
+    def deprecationWarning(msg: => Message, offset: Int = in.offset) =
       ctx.deprecationWarning(msg, source atPos Position(offset))
 
     /** Issue an error at current offset taht input is incomplete */
-    def incompleteInputError(msg: Message) =
+    def incompleteInputError(msg: => Message) =
       ctx.incompleteInputError(msg, source atPos Position(in.offset))
 
     /** If at end of file, issue an incompleteInputError.
      *  Otherwise issue a syntax error and skip to next safe point.
      */
-    def syntaxErrorOrIncomplete(msg: Message) =
+   def syntaxErrorOrIncomplete(msg: => Message) =
       if (in.token == EOF) incompleteInputError(msg)
       else {
         syntaxError(msg)

src/dotty/tools/dotc/reporting/Reporter.scala

@@ -38,16 +38,16 @@ trait Reporting { this: Context =>
     reporter.report(new Info(msg, pos))
 
   def deprecationWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(msg.deprecationWarning(pos))
+    reporter.report(new DeprecationWarning(msg, pos))
 
   def migrationWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(msg.migrationWarning(pos))
+    reporter.report(new MigrationWarning(msg, pos))
 
   def uncheckedWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(msg.uncheckedWarning(pos))
+    reporter.report(new UncheckedWarning(msg, pos))
 
   def featureWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(msg.featureWarning(pos))
+    reporter.report(new FeatureWarning(msg, pos))
 
   def featureWarning(feature: String, featureDescription: String, isScala2Feature: Boolean,
       featureUseSite: Symbol, required: Boolean, pos: SourcePosition): Unit = {
@@ -73,23 +73,27 @@ trait Reporting { this: Context =>
   }
 
   def warning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(msg.warning(pos))
+    reporter.report(new Warning(msg, pos))
 
   def strictWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
     if (this.settings.strict.value) error(msg, pos)
-    else warning(msg.mapMsg(_ + "\n(This would be an error under strict mode)"), pos)
+    else reporter.report {
+      new ExtendMessage(() => msg)(_ + "\n(This would be an error under strict mode)").warning(pos)
+    }
 
   def error(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(msg.error(pos))
+    reporter.report(new Error(msg, pos))
 
   def errorOrMigrationWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
     if (ctx.scala2Mode) migrationWarning(msg, pos) else error(msg, pos)
 
   def restrictionError(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    error(msg.mapMsg(m => s"Implementation restriction: $m"), pos)
+    reporter.report {
+      new ExtendMessage(() => msg)(m => s"Implementation restriction: $m").error(pos)
+    }
 
-  def incompleteInputError(msg: Message, pos: SourcePosition = NoSourcePosition)(implicit ctx: Context): Unit =
-    reporter.incomplete(msg.error(pos))(ctx)
+  def incompleteInputError(msg: => Message, pos: SourcePosition = NoSourcePosition)(implicit ctx: Context): Unit =
+    reporter.incomplete(new Error(msg, pos))(ctx)
 
   /** Log msg if settings.log contains the current phase.
    *  See [[config.CompilerCommand#explainAdvanced]] for the exact meaning of
@@ -192,7 +196,7 @@ trait Reporting { this: Context =>
 abstract class Reporter extends interfaces.ReporterResult {
 
   /** Report a diagnostic */
-  def doReport(d: MessageContainer)(implicit ctx: Context): Unit
+  def doReport(m: MessageContainer)(implicit ctx: Context): Unit
 
   /** Whether very long lines can be truncated.  This exists so important
    *  debugging information (like printing the classpath) is not rendered
@@ -236,22 +240,22 @@ abstract class Reporter extends interfaces.ReporterResult {
     override def default(key: String) = 0
   }
 
-  def report(d: MessageContainer)(implicit ctx: Context): Unit =
-    if (!isHidden(d)) {
-      doReport(d)(ctx.addMode(Mode.Printing))
-      d match {
-        case d: ConditionalWarning if !d.enablingOption.value => unreportedWarnings(d.enablingOption.name) += 1
-        case d: Warning => warningCount += 1
-        case d: Error =>
-          errors = d :: errors
+  def report(m: MessageContainer)(implicit ctx: Context): Unit =
+    if (!isHidden(m)) {
+      doReport(m)(ctx.addMode(Mode.Printing))
+      m match {
+        case m: ConditionalWarning if !m.enablingOption.value => unreportedWarnings(m.enablingOption.name) += 1
+        case m: Warning => warningCount += 1
+        case m: Error =>
+          errors = m :: errors
           errorCount += 1
-        case d: Info => // nothing to do here
+        case m: Info => // nothing to do here
         // match error if d is something else
       }
     }
 
-  def incomplete(d: MessageContainer)(implicit ctx: Context): Unit =
-    incompleteHandler(d)(ctx)
+  def incomplete(m: MessageContainer)(implicit ctx: Context): Unit =
+    incompleteHandler(m)(ctx)
 
   /** Summary of warnings and errors */
   def summary: String = {

src/dotty/tools/dotc/reporting/StoreReporter.scala

@@ -8,9 +8,16 @@ import config.Printers.typr
 import diagnostic.MessageContainer
 import diagnostic.messages._
 
-/**
- * This class implements a Reporter that stores all messages
- */
+/** This class implements a Reporter that stores all messages
+  *
+  * Beware that this reporter can leak memory, and force messages in two
+  * scenarios:
+  *
+  * - During debugging `config.Printers.typr` is set from `noPrinter` to `new
+  *   Printer`, which forces the message
+  * - The reporter is not flushed and the message containers capture a
+  *   `Context` (about 4MB)
+  */
 class StoreReporter(outer: Reporter) extends Reporter {
 
   private var infos: mutable.ListBuffer[MessageContainer] = null
@@ -31,7 +38,7 @@ class StoreReporter(outer: Reporter) extends Reporter {
 
   override def flush()(implicit ctx: Context) =
     if (infos != null) {
-      infos foreach ctx.reporter.report
+      infos.foreach(ctx.reporter.report(_))
       infos = null
     }
 

src/dotty/tools/dotc/reporting/diagnostic/Message.scala

@@ -6,6 +6,8 @@ package diagnostic
 import util.SourcePosition
 import core.Contexts.Context
 
+import messages._
+
 object Message {
   /** This implicit conversion provides a fallback for error messages that have
     * not yet been ported to the new scheme. Comment out this `implicit def` to
@@ -21,6 +23,13 @@ object Message {
   * consumed by a subclass of `Reporter`. However, the error position is only
   * part of `MessageContainer`, not `Message`.
   *
+  * NOTE: you should not be persisting messages. Most messages take an implicit
+  * `Context` and these contexts weigh in at about 4mb per instance, as such
+  * persisting these will result in a memory leak.
+  *
+  * Instead use the `persist` method to create an instance that does not keep a
+  * reference to these contexts.
+  *
   * @param errorId a unique number identifying the message, this will later be
   *                used to reference documentation online
   */
@@ -48,45 +57,62 @@ abstract class Message(val errorId: Int) { self =>
     */
   def explanation: String
 
-  /** It is possible to map `msg` to add details, this is at the loss of
-    * precision since the type of the resulting `Message` won't be original
-    * extending class
-    *
-    * @return a `Message` with the mapped message
+  /** The implicit `Context` in messages is a large thing that we don't want
+    * persisted. This method gets around that by duplicating the message
+    * without the implicit context being passed along.
     */
-  def mapMsg(f: String => String) = new Message(errorId) {
-    val msg = f(self.msg)
+  def persist: Message = new Message (errorId) {
+    val msg         = self.msg
+    val kind        = self.kind
+    val explanation = self.explanation
+  }
+}
+
+/** An extended message keeps the contained message from being evaluated, while
+  * allowing for extension for the `msg` string
+  *
+  * This is useful when we need to add additional information to an existing
+  * message.
+  */
+class ExtendMessage(_msg: () => Message)(f: String => String) { self =>
+  lazy val msg = f(_msg().msg)
+  lazy val kind = _msg().kind
+  lazy val explanation = _msg().explanation
+  lazy val errorId = _msg().errorId
+
+  private def toMessage = new Message(errorId) {
+    val msg = self.msg
     val kind = self.kind
     val explanation = self.explanation
   }
 
   /** Enclose this message in an `Error` container */
   def error(pos: SourcePosition) =
-    new Error(self, pos)
+    new Error(toMessage, pos)
 
   /** Enclose this message in an `Warning` container */
   def warning(pos: SourcePosition) =
-    new Warning(self, pos)
+    new Warning(toMessage, pos)
 
   /** Enclose this message in an `Info` container */
   def info(pos: SourcePosition) =
-    new Info(self, pos)
+    new Info(toMessage, pos)
 
   /** Enclose this message in an `FeatureWarning` container */
   def featureWarning(pos: SourcePosition) =
-    new FeatureWarning(self, pos)
+    new FeatureWarning(toMessage, pos)
 
   /** Enclose this message in an `UncheckedWarning` container */
   def uncheckedWarning(pos: SourcePosition) =
-    new UncheckedWarning(self, pos)
+    new UncheckedWarning(toMessage, pos)
 
   /** Enclose this message in an `DeprecationWarning` container */
   def deprecationWarning(pos: SourcePosition) =
-    new DeprecationWarning(self, pos)
+    new DeprecationWarning(toMessage, pos)
 
   /** Enclose this message in an `MigrationWarning` container */
   def migrationWarning(pos: SourcePosition) =
-    new MigrationWarning(self, pos)
+    new MigrationWarning(toMessage, pos)
 }
 
 /** The fallback `Message` containing no explanation and having no `kind` */

src/dotty/tools/dotc/reporting/diagnostic/messages.scala

@@ -284,7 +284,7 @@ object messages {
     val explanation = ""
   }
 
-  case class EarlyDefinitionsNotSupported()(implicit ctx:Context)
+  case class EarlyDefinitionsNotSupported()(implicit ctx: Context)
   extends Message(9) {
     val kind = "Syntax"
     val msg = "early definitions are not supported; use trait parameters instead"

test/dotty/tools/dotc/reporting/TestMessageLaziness.scala

@@ -0,0 +1,37 @@
+package dotty.tools
+package dotc
+package reporting
+
+import org.junit.Assert._
+import org.junit.Test
+
+import core.Contexts._
+
+import test.DottyTest
+
+import diagnostic.{ Message, MessageContainer, ExtendMessage }
+
+class TestMessageLaziness extends DottyTest {
+  ctx = ctx.fresh.setReporter(new NonchalantReporter)
+
+  class NonchalantReporter(implicit ctx: Context) extends Reporter
+  with UniqueMessagePositions with HideNonSensicalMessages {
+    def doReport(m: MessageContainer)(implicit ctx: Context) = ???
+
+    override def report(m: MessageContainer)(implicit ctx: Context) = ()
+  }
+
+  case class LazyError() extends Message(1000) {
+    throw new Error("Didn't stay lazy.")
+
+    val kind = "Test"
+    val msg = "Please don't blow up"
+    val explanation = ""
+  }
+
+  @Test def assureLazy =
+    ctx.error(LazyError())
+
+  @Test def assureLazyExtendMessage =
+    ctx.strictWarning(LazyError())
+}