Skip to content

Improve and document the Driver#process API, fix partest logging #1052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 4, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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()(ctx)

def reset()(implicit ctx: Context): Unit = {
ctx.base.reset()
ctx.runInfo.clear()
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/CompilerCallback.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
68 changes: 52 additions & 16 deletions src/dotty/tools/dotc/Driver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,28 +33,64 @@ 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)
}

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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the default arguments here? Or do the overloaded variants of process cover all the use cases already?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, yes, I don't want to add even more overloads like:

def process(args: Array[String], reporter: Reporter)
def process(args: Array[String], callback: CompilerCallback)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, fair enough.

On Thu, Feb 4, 2016 at 7:09 PM, Guillaume Martres notifications@github.com
wrote:

In src/dotty/tools/dotc/Driver.scala
#1052 (comment):

  • 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,

I think so, yes, I don't want to add even more overloads like:

def process(args: Array[String], reporter: Reporter)def process(args: Array[String], callback: CompilerCallback)


Reply to this email directly or view it on GitHub
https://github.com/lampepfl/dotty/pull/1052/files#r51912246.

Martin Odersky
EPFL

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expects

* 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 = {
Expand Down
24 changes: 8 additions & 16 deletions src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 = _
Expand Down Expand Up @@ -426,7 +420,9 @@ 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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a useful helper method. I think there are several occurrences of setTyperState(ctx.typerState.withReporter(...)) which would profit from that method. Best to do it now.

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 }
Expand Down Expand Up @@ -481,7 +477,7 @@ object Contexts {
outer = NoContext
period = InitialPeriod
mode = Mode.None
typerState = new TyperState(new ThrowingReporter(new ConsoleReporter()(this)))
typerState = new TyperState(new ConsoleReporter())
printerFn = new RefinedPrinter(_)
owner = NoSymbol
sstate = settings.defaultState
Expand Down Expand Up @@ -536,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 */
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/reporting/ConsoleReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
21 changes: 16 additions & 5 deletions src/dotty/tools/dotc/reporting/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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". */
Expand Down
3 changes: 1 addition & 2 deletions src/dotty/tools/dotc/transform/TreeChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 4 additions & 14 deletions test/dotty/partest/DPDirectCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,22 @@ 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 = {
val clogFWriter = new FileWriter(runner.cLogFile.jfile, true)
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
import base.settings._
val ctx = base.initialCtx.fresh.setSetting(printtypes, true)
.setSetting(pageWidth, 90).setSetting(log, List("<some"))
base.definitions.init(ctx)
ctx
}

try {
val processor =
if (opts0.exists(_.startsWith("#"))) dotty.tools.dotc.Bench else dotty.tools.dotc.Main
val clogger = new ConsoleReporter(writer = clogWriter)(ctx)
val logCtx = ctx.fresh.setTyperState(ctx.typerState.withReporter(clogger))
val reporter = processor.process((sources.map(_.toString) ::: opts0).toArray, logCtx)
val clogger = new ConsoleReporter(writer = clogWriter)
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 {
Expand Down
4 changes: 2 additions & 2 deletions test/test/CompilerTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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/ */
Expand Down Expand Up @@ -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")
}

Expand Down
11 changes: 0 additions & 11 deletions test/test/DottyTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,6 @@ class DottyTest /*extends ContextEscapeDetection*/ {
val base = new ContextBase
import base.settings._
val ctx = base.initialCtx.fresh
//.setSetting(verbose, true)
// .withSetting(debug, true)
// .withSetting(debugTrace, true)
// .withSetting(prompt, true)
//.setSetting(Ylogcp, true)
//.setSetting(printtypes, true)
.setSetting(pageWidth, 90)
.setSetting(log, List("<some"))
// .withTyperState(new TyperState(new ConsoleReporter()(base.initialCtx)))

// .withSetting(uniqid, true)
base.definitions.init(ctx)
ctx
}
Expand Down