From cf1413bfc89308aa4b44aa1b76019c168c32a343 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 3 Feb 2016 20:49:15 +0100 Subject: [PATCH 1/8] ConsoleReporter: remove unused argument ctx --- src/dotty/tools/dotc/Compiler.scala | 2 +- src/dotty/tools/dotc/core/Contexts.scala | 2 +- src/dotty/tools/dotc/reporting/ConsoleReporter.scala | 2 +- test/dotty/partest/DPDirectCompiler.scala | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index 42d223fe9e83..f1ef7afc34d0 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -118,7 +118,7 @@ class Compiler { (start.setRunInfo(new RunInfo(start)) /: defn.RootImportFns)(addImport) } - protected def rootReporter(implicit ctx: Context): Reporter = new ConsoleReporter()(ctx) + protected def rootReporter(implicit ctx: Context): Reporter = new ConsoleReporter() def reset()(implicit ctx: Context): Unit = { ctx.base.reset() diff --git a/src/dotty/tools/dotc/core/Contexts.scala b/src/dotty/tools/dotc/core/Contexts.scala index ae221cc3e3da..a5a0f4d65f55 100644 --- a/src/dotty/tools/dotc/core/Contexts.scala +++ b/src/dotty/tools/dotc/core/Contexts.scala @@ -481,7 +481,7 @@ object Contexts { outer = NoContext period = InitialPeriod mode = Mode.None - typerState = new TyperState(new ThrowingReporter(new ConsoleReporter()(this))) + typerState = new TyperState(new ThrowingReporter(new ConsoleReporter())) printerFn = new RefinedPrinter(_) owner = NoSymbol sstate = settings.defaultState diff --git a/src/dotty/tools/dotc/reporting/ConsoleReporter.scala b/src/dotty/tools/dotc/reporting/ConsoleReporter.scala index 45268b6735fe..e9b9964c3eb0 100644 --- a/src/dotty/tools/dotc/reporting/ConsoleReporter.scala +++ b/src/dotty/tools/dotc/reporting/ConsoleReporter.scala @@ -15,7 +15,7 @@ import scala.reflect.internal.util._ */ class ConsoleReporter( reader: BufferedReader = Console.in, - writer: PrintWriter = new PrintWriter(Console.err, true))(ctx: Context) + writer: PrintWriter = new PrintWriter(Console.err, true)) extends Reporter with UniqueMessagePositions { /** maximal number of error messages to be printed */ diff --git a/test/dotty/partest/DPDirectCompiler.scala b/test/dotty/partest/DPDirectCompiler.scala index ca56ac3e991e..f08af24a936c 100644 --- a/test/dotty/partest/DPDirectCompiler.scala +++ b/test/dotty/partest/DPDirectCompiler.scala @@ -25,7 +25,7 @@ class DPDirectCompiler(runner: DPTestRunner) extends nest.DirectCompiler(runner) try { val processor = if (opts0.exists(_.startsWith("#"))) dotty.tools.dotc.Bench else dotty.tools.dotc.Main - val clogger = new ConsoleReporter(writer = clogWriter)(ctx) + val clogger = new ConsoleReporter(writer = clogWriter) val logCtx = ctx.fresh.setTyperState(ctx.typerState.withReporter(clogger)) val reporter = processor.process((sources.map(_.toString) ::: opts0).toArray, logCtx) if (!reporter.hasErrors) runner.genPass() From d6a52eec2221101973c6b28f54aa20319f0e8b6f Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 3 Feb 2016 21:12:37 +0100 Subject: [PATCH 2/8] Reporter: make summary available without a Context --- src/dotty/tools/dotc/reporting/Reporter.scala | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/dotty/tools/dotc/reporting/Reporter.scala b/src/dotty/tools/dotc/reporting/Reporter.scala index f98d85ce9d7f..5ed7360da3a4 100644 --- a/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/src/dotty/tools/dotc/reporting/Reporter.scala @@ -247,12 +247,23 @@ abstract class Reporter { incompleteHandler(d)(ctx) - /** Print a summary */ - def printSummary(implicit ctx: Context): Unit = { - if (warningCount > 0) ctx.println(countString(warningCount, "warning") + " found") - if (errorCount > 0) ctx.println(countString(errorCount, "error") + " found") + /** Summary of warnings and errors */ + def summary: String = { + val b = new mutable.ListBuffer[String] + if (warningCount > 0) + b += countString(warningCount, "warning") + " found" + if (errorCount > 0) + b += countString(errorCount, "error") + " found" for ((settingName, count) <- unreportedWarnings) - ctx.println(s"there were $count ${settingName.tail} warning(s); re-run with $settingName for details") + b += s"there were $count ${settingName.tail} warning(s); re-run with $settingName for details" + b.mkString("\n") + } + + /** Print the summary of warnings and errors */ + def printSummary(implicit ctx: Context): Unit = { + val s = summary + if (s != "") + ctx.println(s) } /** Returns a string meaning "n elements". */ From f926c8c14a7b48f2c0f3da680c69881ddfb9bf46 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 3 Feb 2016 20:51:22 +0100 Subject: [PATCH 3/8] Remove useless options in tests - Removed "-Xprint-types", it is only rarely needed and makes it very hard to read trees, enable it yourself if you need it. - Removed "-Ylog: Date: Wed, 3 Feb 2016 22:42:36 +0100 Subject: [PATCH 4/8] Driver: properly use root Context passed as argument Also CompilerTest no longer runs the compiler with the context DottyTest#ctx. Previously, we got away with this because Compiler#process ignored it and created a new Context, but this commit fixes this, and it is now very important that we use a different context for every test we compile. Since DottyTest#ctx was the only part of DottyTest we used, CompilerTest no longer extends DottyTest to make sure that we do not use it accidentally. If we want to use DottyTest as a base class for tests again, we will have to remove its implicit Context field first. Also do not try to initialize the definitions in the context used by partest, this is not necessary. --- src/dotty/tools/dotc/Driver.scala | 10 ++++------ test/dotty/partest/DPDirectCompiler.scala | 6 ++---- test/test/CompilerTest.scala | 4 ++-- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/dotty/tools/dotc/Driver.scala b/src/dotty/tools/dotc/Driver.scala index 1627b6e4822d..57fa4a052602 100644 --- a/src/dotty/tools/dotc/Driver.scala +++ b/src/dotty/tools/dotc/Driver.scala @@ -33,12 +33,10 @@ abstract class Driver extends DotClass { protected def sourcesRequired = true def setup(args: Array[String], rootCtx: Context): (List[String], Context) = { - val summary = CompilerCommand.distill(args)(rootCtx) - // FIXME: We should reuse rootCtx instead of creating newCtx, but this - // makes some tests fail with "denotation module _root_ invalid in run 2." - val newCtx = initCtx.setCompilerCallback(rootCtx.compilerCallback) - implicit val ctx: Context = newCtx.fresh.setSettings(summary.sstate) - val fileNames = CompilerCommand.checkUsage(summary, sourcesRequired) + val ctx = rootCtx.fresh + val summary = CompilerCommand.distill(args)(ctx) + ctx.setSettings(summary.sstate) + val fileNames = CompilerCommand.checkUsage(summary, sourcesRequired)(ctx) (fileNames, ctx) } diff --git a/test/dotty/partest/DPDirectCompiler.scala b/test/dotty/partest/DPDirectCompiler.scala index a6f11ec64fff..4e563820fb07 100644 --- a/test/dotty/partest/DPDirectCompiler.scala +++ b/test/dotty/partest/DPDirectCompiler.scala @@ -5,7 +5,7 @@ import scala.tools.partest.{ TestState, nest } import java.io.{ File, PrintWriter, FileWriter } -/* NOTE: Adapted from partest.DirectCompiler and DottyTest */ +/* NOTE: Adapted from partest.DirectCompiler */ class DPDirectCompiler(runner: DPTestRunner) extends nest.DirectCompiler(runner) { override def compile(opts0: List[String], sources: List[File]): TestState = { @@ -15,9 +15,7 @@ class DPDirectCompiler(runner: DPTestRunner) extends nest.DirectCompiler(runner) implicit val ctx: dotty.tools.dotc.core.Contexts.Context = { val base = new dotty.tools.dotc.core.Contexts.ContextBase - val ctx = base.initialCtx.fresh - base.definitions.init(ctx) - ctx + base.initialCtx.fresh } try { diff --git a/test/test/CompilerTest.scala b/test/test/CompilerTest.scala index c65710e7d718..5263925765cf 100644 --- a/test/test/CompilerTest.scala +++ b/test/test/CompilerTest.scala @@ -33,7 +33,7 @@ import org.junit.Test * object Test { def main(args: Array[String]): Unit = ... } * Classpath jars can be added to partestDeps in the sbt Build.scala. */ -abstract class CompilerTest extends DottyTest { +abstract class CompilerTest { /** Override with output dir of test so it can be patched. Partest expects * classes to be in partest-generated/[kind]/[testname]-[kind].obj/ */ @@ -181,7 +181,7 @@ abstract class CompilerTest extends DottyTest { private def compileArgs(args: Array[String], xerrors: Int = 0)(implicit defaultOptions: List[String]): Unit = { val allArgs = args ++ defaultOptions val processor = if (allArgs.exists(_.startsWith("#"))) Bench else Main - val nerrors = processor.process(allArgs, ctx).errorCount + val nerrors = processor.process(allArgs).errorCount assert(nerrors == xerrors, s"Wrong # of errors. Expected: $xerrors, found: $nerrors") } From 208f7cde44a06947e452ba6c5132704caa4b1c3e Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 4 Feb 2016 02:37:40 +0100 Subject: [PATCH 5/8] Compiler: use the reporter passed from the Driver With this commit and the previous one, partest should finally correctly log the output of the compiler and display it at the end of its execution if compilation failed. Also make sure the initial Reporter in a Context is _not_ a ThrowingReporter. Previously, the initial Reporter was always overriden in Compiler by rootReporter so it had no effect, but this is not the case anymore, and the negative junit tests fail with the ThrowingReporter since they throw an exception instead of exiting with a certain number of errors. --- src/dotty/tools/dotc/Compiler.scala | 4 +--- src/dotty/tools/dotc/core/Contexts.scala | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index f1ef7afc34d0..19965786460d 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -111,15 +111,13 @@ class Compiler { .setOwner(defn.RootClass) .setTyper(new Typer) .setMode(Mode.ImplicitsEnabled) - .setTyperState(new MutableTyperState(ctx.typerState, rootReporter(ctx), isCommittable = true)) + .setTyperState(new MutableTyperState(ctx.typerState, ctx.typerState.reporter, isCommittable = true)) ctx.definitions.init(start) // set context of definitions to start def addImport(ctx: Context, refFn: () => TermRef) = ctx.fresh.setImportInfo(ImportInfo.rootImport(refFn)(ctx)) (start.setRunInfo(new RunInfo(start)) /: defn.RootImportFns)(addImport) } - protected def rootReporter(implicit ctx: Context): Reporter = new ConsoleReporter() - def reset()(implicit ctx: Context): Unit = { ctx.base.reset() ctx.runInfo.clear() diff --git a/src/dotty/tools/dotc/core/Contexts.scala b/src/dotty/tools/dotc/core/Contexts.scala index a5a0f4d65f55..14e8ccb52590 100644 --- a/src/dotty/tools/dotc/core/Contexts.scala +++ b/src/dotty/tools/dotc/core/Contexts.scala @@ -481,7 +481,7 @@ object Contexts { outer = NoContext period = InitialPeriod mode = Mode.None - typerState = new TyperState(new ThrowingReporter(new ConsoleReporter())) + typerState = new TyperState(new ConsoleReporter()) printerFn = new RefinedPrinter(_) owner = NoSymbol sstate = settings.defaultState From 7827c5ae1744583974f9c99432c1082ea6cb8997 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 3 Feb 2016 23:27:49 +0100 Subject: [PATCH 6/8] Better compiler entry points - Document the entry points - It is now possible to set a custom reporter without using a custom context - Use `null` for optional arguments to make it easier to run the compiler using reflection or from Java. - DPDirectCompiler does not use a custom context anymore --- src/dotty/tools/dotc/Driver.scala | 58 +++++++++++++++++++---- src/dotty/tools/dotc/core/Contexts.scala | 1 + test/dotty/partest/DPDirectCompiler.scala | 10 +--- 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/dotty/tools/dotc/Driver.scala b/src/dotty/tools/dotc/Driver.scala index 57fa4a052602..7f22fc77465f 100644 --- a/src/dotty/tools/dotc/Driver.scala +++ b/src/dotty/tools/dotc/Driver.scala @@ -40,19 +40,57 @@ abstract class Driver extends DotClass { (fileNames, ctx) } - def process(args: Array[String], rootCtx: Context): Reporter = { - val (fileNames, ctx) = setup(args, rootCtx) - doCompile(newCompiler(), fileNames)(ctx) - } - def process(args: Array[String], callback: CompilerCallback): Reporter = { - process(args, initCtx.setCompilerCallback(callback)) + /** Principal entry point to the compiler. + * Creates a new compiler instance and run it with arguments `args`. + * + * The optional arguments of this method all have `null` as their default + * value, this makes it easier to call this method by reflection or from Java. + * + * @param args Arguments to pass to the compiler. + * @param reporter Used to log errors, warnings, and info messages. + * The default reporter is used if this is `null`. + * @param callback Used to execute custom code during the compilation + * process. No callbacks will be executed if this is `null`. + * @return The `Reporter` used. Use `Reporter#hasErrors` to check + * if compilation succeeded. + */ + final def process(args: Array[String], reporter: Reporter = null, + callback: CompilerCallback = null): Reporter = { + val ctx = initCtx.fresh + if (reporter != null) + ctx.setReporter(reporter) + if (callback != null) + ctx.setCompilerCallback(callback) + process(args, ctx) } - // We overload `process` instead of using a default argument so that we - // can easily call this method using reflection from `RawCompiler` in sbt. - def process(args: Array[String]): Reporter = { - process(args, initCtx) + /** Entry point to the compiler with no optional arguments. + * + * This overload is provided for compatibility reasons: the + * `RawCompiler` of sbt expects this method to exist and calls + * it using reflection. Keeping it means that we can change + * the other overloads without worrying about breaking compatibility + * with sbt. + */ + final def process(args: Array[String]): Reporter = + process(args, null, null) + + /** Entry point to the compiler using a custom `Context`. + * + * In most cases, you do not need a custom `Context` and should + * instead use one of the other overloads of `process`. However, + * the other overloads cannot be overriden, instead you + * should override this one which they call internally. + * + * @param args Arguments to pass to the compiler. + * @param rootCtx The root Context to use. + * @return The `Reporter` used. Use `Reporter#hasErrors` to check + * if compilation succeeded. + */ + def process(args: Array[String], rootCtx: Context): Reporter = { + val (fileNames, ctx) = setup(args, rootCtx) + doCompile(newCompiler(), fileNames)(ctx) } def main(args: Array[String]): Unit = { diff --git a/src/dotty/tools/dotc/core/Contexts.scala b/src/dotty/tools/dotc/core/Contexts.scala index 14e8ccb52590..f7af6f43eb9a 100644 --- a/src/dotty/tools/dotc/core/Contexts.scala +++ b/src/dotty/tools/dotc/core/Contexts.scala @@ -427,6 +427,7 @@ object Contexts { def setPeriod(period: Period): this.type = { this.period = period; this } def setMode(mode: Mode): this.type = { this.mode = mode; this } def setTyperState(typerState: TyperState): this.type = { this.typerState = typerState; this } + def setReporter(reporter: Reporter): this.type = setTyperState(typerState.withReporter(reporter)) def setNewTyperState: this.type = setTyperState(typerState.fresh(isCommittable = true)) def setExploreTyperState: this.type = setTyperState(typerState.fresh(isCommittable = false)) def setPrinterFn(printer: Context => Printer): this.type = { this.printerFn = printer; this } diff --git a/test/dotty/partest/DPDirectCompiler.scala b/test/dotty/partest/DPDirectCompiler.scala index 4e563820fb07..410dac338df2 100644 --- a/test/dotty/partest/DPDirectCompiler.scala +++ b/test/dotty/partest/DPDirectCompiler.scala @@ -13,20 +13,14 @@ class DPDirectCompiler(runner: DPTestRunner) extends nest.DirectCompiler(runner) val clogWriter = new PrintWriter(clogFWriter, true) clogWriter.println("\ncompiling " + sources.mkString(" ") + "\noptions: " + opts0.mkString(" ")) - implicit val ctx: dotty.tools.dotc.core.Contexts.Context = { - val base = new dotty.tools.dotc.core.Contexts.ContextBase - base.initialCtx.fresh - } - try { val processor = if (opts0.exists(_.startsWith("#"))) dotty.tools.dotc.Bench else dotty.tools.dotc.Main val clogger = new ConsoleReporter(writer = clogWriter) - val logCtx = ctx.fresh.setTyperState(ctx.typerState.withReporter(clogger)) - val reporter = processor.process((sources.map(_.toString) ::: opts0).toArray, logCtx) + val reporter = processor.process((sources.map(_.toString) ::: opts0).toArray, clogger) if (!reporter.hasErrors) runner.genPass() else { - reporter.printSummary(ctx) + clogWriter.println(reporter.summary) runner.genFail(s"compilation failed with ${reporter.errorCount} errors") } } catch { From a33d517f4b3c00070b2e66afb82d2cc4284d4894 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 3 Feb 2016 23:37:11 +0100 Subject: [PATCH 7/8] Context: set compilerCallback like other context variables Previously, we could set compilerCallback on non-fresh contexts, but there is no reason that this should be allowed, and this is not done anymore in the code since the last commit. --- src/dotty/tools/dotc/CompilerCallback.scala | 2 +- src/dotty/tools/dotc/core/Contexts.scala | 21 ++++++--------------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/src/dotty/tools/dotc/CompilerCallback.scala b/src/dotty/tools/dotc/CompilerCallback.scala index 98e8e2713dbd..e2f56430b3aa 100644 --- a/src/dotty/tools/dotc/CompilerCallback.scala +++ b/src/dotty/tools/dotc/CompilerCallback.scala @@ -18,7 +18,7 @@ import java.io.File * } * dotty.tools.dotc.process(args, callback) * // Or, if you have a custom root context `rootCtx`: - * dotty.tools.dotc.process(args, rootCtx.setCompilerCallback(callback)) + * dotty.tools.dotc.process(args, rootCtx.fresh.setCompilerCallback(callback)) * }}} */ trait CompilerCallback { diff --git a/src/dotty/tools/dotc/core/Contexts.scala b/src/dotty/tools/dotc/core/Contexts.scala index f7af6f43eb9a..b205a40f00bd 100644 --- a/src/dotty/tools/dotc/core/Contexts.scala +++ b/src/dotty/tools/dotc/core/Contexts.scala @@ -72,22 +72,16 @@ object Contexts { def next = { val c = current; current = current.outer; c } } - /** Set the compiler callback, shared by all contexts with the same `base` */ - def setCompilerCallback(callback: CompilerCallback): this.type = { - base.compilerCallback = callback; this - } - /** The outer context */ private[this] var _outer: Context = _ protected def outer_=(outer: Context) = _outer = outer def outer: Context = _outer - // protected def compilerCallback_=(callback: CompilerCallback) = - // _compilerCallback = callback - // def compilerCallback: CompilerCallback = _compilerCallback - // def setCompilerCallback(callback: CompilerCallback): this.type = { - // this.compilerCallback = callback; this - // } + /** The compiler callback implementation, or null if no callback will be called. */ + private[this] var _compilerCallback: CompilerCallback = _ + protected def compilerCallback_=(callback: CompilerCallback) = + _compilerCallback = callback + def compilerCallback: CompilerCallback = _compilerCallback /** The current context */ private[this] var _period: Period = _ @@ -426,6 +420,7 @@ object Contexts { abstract class FreshContext extends Context { def setPeriod(period: Period): this.type = { this.period = period; this } def setMode(mode: Mode): this.type = { this.mode = mode; this } + def setCompilerCallback(callback: CompilerCallback): this.type = { this.compilerCallback = callback; this } def setTyperState(typerState: TyperState): this.type = { this.typerState = typerState; this } def setReporter(reporter: Reporter): this.type = setTyperState(typerState.withReporter(reporter)) def setNewTyperState: this.type = setTyperState(typerState.fresh(isCommittable = true)) @@ -537,10 +532,6 @@ object Contexts { /** The essential mutable state of a context base, collected into a common class */ class ContextState { - - /** The compiler callback implementation, or null if unset */ - var compilerCallback: CompilerCallback = _ - // Symbols state /** A counter for unique ids */ From 7eba7f7a6778cc0ddfb2ce81dee64dd4fa23490a Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 4 Feb 2016 19:24:59 +0100 Subject: [PATCH 8/8] Use Context#setReporter where possible --- src/dotty/tools/dotc/transform/TreeChecker.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/TreeChecker.scala b/src/dotty/tools/dotc/transform/TreeChecker.scala index daf76f4716aa..150a632a1476 100644 --- a/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -118,8 +118,7 @@ class TreeChecker extends Phase with SymTransformer { val prevPhase = ctx.phase.prev // can be a mini-phase val squahsedPhase = ctx.squashed(prevPhase) ctx.println(s"checking ${ctx.compilationUnit} after phase ${squahsedPhase}") - val checkingCtx = ctx.fresh - .setTyperState(ctx.typerState.withReporter(new ThrowingReporter(ctx.reporter))) + val checkingCtx = ctx.fresh.setReporter(new ThrowingReporter(ctx.reporter)) val checker = new Checker(previousPhases(phasesToRun.toList)(ctx)) try checker.typedExpr(ctx.compilationUnit.tpdTree)(checkingCtx) catch {