diff options
author | Felix Mulder <felix.mulder@gmail.com> | 2016-11-17 10:08:49 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-11-17 10:08:49 +0100 |
commit | a0169b7eda8bf68b17c59701838017099f31d239 (patch) | |
tree | f2a4e1f11cf58665e6d2927376392cf33fb61d68 | |
parent | 528bccf9de97dd83418780bf26b3e797dde59875 (diff) | |
parent | a2354dd7e347a191c6077105b136d683013dd092 (diff) | |
download | dotty-a0169b7eda8bf68b17c59701838017099f31d239.tar.gz dotty-a0169b7eda8bf68b17c59701838017099f31d239.tar.bz2 dotty-a0169b7eda8bf68b17c59701838017099f31d239.zip |
Merge pull request #1696 from felixmulder/topic/assert-message-laziness
Make sure messages are lazily evaluated until `report` in `Reporter`
6 files changed, 122 insertions, 48 deletions
diff --git a/src/dotty/tools/dotc/parsing/Parsers.scala b/src/dotty/tools/dotc/parsing/Parsers.scala index 3fb6de262..bb76de6cc 100644 --- a/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/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) diff --git a/src/dotty/tools/dotc/reporting/Reporter.scala b/src/dotty/tools/dotc/reporting/Reporter.scala index 49bd3e811..8477cfe28 100644 --- a/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/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 = { diff --git a/src/dotty/tools/dotc/reporting/StoreReporter.scala b/src/dotty/tools/dotc/reporting/StoreReporter.scala index e85017ed2..586273c2e 100644 --- a/src/dotty/tools/dotc/reporting/StoreReporter.scala +++ b/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 } diff --git a/src/dotty/tools/dotc/reporting/diagnostic/Message.scala b/src/dotty/tools/dotc/reporting/diagnostic/Message.scala index 8b607c18c..2497fb216 100644 --- a/src/dotty/tools/dotc/reporting/diagnostic/Message.scala +++ b/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` */ diff --git a/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 5e6a402ef..446c0e0c7 100644 --- a/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/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" diff --git a/test/dotty/tools/dotc/reporting/TestMessageLaziness.scala b/test/dotty/tools/dotc/reporting/TestMessageLaziness.scala new file mode 100644 index 000000000..6892739e8 --- /dev/null +++ b/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()) +} |