Make sure messages are lazily evaluated until report in Reporter

Author: felixmulder

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)
+    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
@@ -236,7 +240,7 @@ abstract class Reporter extends interfaces.ReporterResult {
     override def default(key: String) = 0
   }
 
-  def report(d: MessageContainer)(implicit ctx: Context): Unit =
+  def report(d: => MessageContainer)(implicit ctx: Context): Unit =
     if (!isHidden(d)) {
       doReport(d)(ctx.addMode(Mode.Printing))
       d match {

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

@@ -31,7 +31,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
@@ -20,6 +22,13 @@ object Message {
   * into a `MessageContainer` which contains the log level and can later be
   * consumed by a subclass of `Reporter`.
   *
+  * 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
   */
@@ -47,45 +56,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

@@ -282,7 +282,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(d: MessageContainer)(implicit ctx: Context) = ???
+
+    override def report(d: => 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())
+}