Skip to content

WIP: Using Silent case class to support suppressing output of repl commands #3048

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

Closed
wants to merge 2 commits into from
Closed
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
2 changes: 1 addition & 1 deletion collection-strawman
7 changes: 6 additions & 1 deletion compiler/src/dotty/tools/repl/ParseResult.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,13 @@ import dotc.reporting._

import results._

sealed trait Parsing

/** A parsing result from string input */
sealed trait ParseResult
sealed trait ParseResult extends Parsing

/** Suppress visual output when this is passed */
case class Silent(underlying: ParseResult) extends Parsing

/** An error free parsing resulting in a list of untyped trees */
case class Parsed(sourceCode: String, trees: List[untpd.Tree]) extends ParseResult
Expand Down
177 changes: 103 additions & 74 deletions compiler/src/dotty/tools/repl/ReplDriver.scala
Original file line number Diff line number Diff line change
@@ -1,33 +1,31 @@
package dotty.tools
package repl

import java.io.{ InputStream, PrintStream }
import java.io.{InputStream, PrintStream}

import scala.annotation.tailrec

import dotc.reporting.MessageRendering
import dotc.reporting.diagnostic.MessageContainer
import dotc.ast.untpd
import dotc.ast.tpd
import dotc.interactive.{ SourceTree, Interactive }
import dotc.interactive.{Interactive, SourceTree}
import dotc.core.Contexts.Context
import dotc.{ CompilationUnit, Run }
import dotc.core.Mode
import dotc.{CompilationUnit, Run}
import dotc.core.{Denotations, Mode}
import dotc.core.Flags._
import dotc.core.Types._
import dotc.core.StdNames._
import dotc.core.Names.Name
import dotc.core.NameOps._
import dotc.core.Symbols.{ Symbol, NoSymbol, defn }
import dotc.core.Symbols.{NoSymbol, Symbol, defn}
import dotc.core.Denotations.Denotation
import dotc.core.Types.{ ExprType, ConstantType }
import dotc.core.Types.{ConstantType, ExprType}
import dotc.core.NameKinds.SimpleNameKind
import dotc.config.CompilerCommand
import dotc.{ Compiler, Driver }
import dotc.{Compiler, Driver}
import dotc.printing.SyntaxHighlighting
import dotc.util.Positions.Position
import io._

import AmmoniteReader._
import results._

Expand Down Expand Up @@ -79,7 +77,9 @@ case class Completions(cursor: Int,
/** Main REPL instance, orchestrating input, compilation and presentation */
class ReplDriver(settings: Array[String],
protected val out: PrintStream = System.out,
protected val classLoader: Option[ClassLoader] = None) extends Driver {
protected val classLoader: Option[ClassLoader] = None,
initialCommands: Option[String] = None,
cleanupCommands: Option[String] = None) extends Driver {

/** Overridden to `false` in order to not have to give sources on the
* commandline
Expand Down Expand Up @@ -124,24 +124,30 @@ class ReplDriver(settings: Array[String],
resetToInitial()

/** Run REPL with `state` until `:quit` command found
*
* This method is the main entry point into the REPL. Its effects are not
* observable outside of the CLI, for this reason, most helper methods are
* `protected final` to facilitate testing.
*/
@tailrec final def runUntilQuit(state: State = initState): State = {
val res = readLine()(state)

if (res == Quit) {
out.println()
state
}
else {
// readLine potentially destroys the run, so a new one is needed for the
// rest of the interpretation:
implicit val freshState = state.newRun(compiler, rootCtx)
runUntilQuit(interpret(res))
*
* This method is the main entry point into the REPL. Its effects are not
* observable outside of the CLI, for this reason, most helper methods are
* `protected final` to facilitate testing.
*/
final def runUntilQuit(initialState: State = initState): State = {
@tailrec def run(state: State = initState): State = {
val res = readLine()(state)

if (res == Quit) {
out.println()
state
}
else {
// readLine potentially destroys the run, so a new one is needed for the
// rest of the interpretation:
implicit val freshState = state.newRun(compiler, rootCtx)
run(interpret(res))
}
}

val state = runBootstrapCommands(initialCommands)(initialState)
val userState = run(state)
runBootstrapCommands(cleanupCommands)(userState)
}

final def run(input: String)(implicit state: State): State =
Expand All @@ -150,6 +156,12 @@ class ReplDriver(settings: Array[String],
final def run(res: ParseResult)(implicit state: State): State =
interpret(res)

final def runBootstrapCommands(cmds: Option[String])(implicit state: State): State = {
cmds.map(ParseResult.apply(_)(rootCtx)).map(Silent.apply(_)).foldLeft(state) { (s, cmd) =>
interpret(cmd)(s)
}
}

/** Extract possible completions at the index of `cursor` in `expr` */
protected[this] final def completions(cursor: Int, expr: String, state0: State): Completions = {
// TODO move some of this logic to `Interactive`
Expand Down Expand Up @@ -184,28 +196,34 @@ class ReplDriver(settings: Array[String],
private def extractImports(trees: List[untpd.Tree])(implicit context: Context): List[(untpd.Import, String)] =
trees.collect { case imp: untpd.Import => (imp, imp.show) }

private def interpret(res: ParseResult)(implicit state: State): State =
res match {
private def interpret(res: Parsing)(implicit state: State): State = {
val (parseResult, isSilent) = res match {
case Silent(x) => (x, true)
case x: ParseResult => (x, false)
}

parseResult match {
case parsed: Parsed if parsed.trees.nonEmpty =>
compile(parsed)
compile(parsed, isSilent)
.withHistory(parsed.sourceCode :: state.history)
.newRun(compiler, rootCtx)

case SyntaxErrors(src, errs, _) =>
displayErrors(errs)
state.withHistory(src :: state.history)

case cmd: Command => interpretCommand(cmd)
case cmd: Command => interpretCommand(cmd, isSilent)

case SigKill => // TODO
state

case _ => // new line, empty tree
state
}
}

/** Compile `parsed` trees and evolve `state` in accordance */
protected[this] final def compile(parsed: Parsed)(implicit state: State): State = {
protected[this] final def compile(parsed: Parsed, silent: Boolean = false)(implicit state: State): State = {
import dotc.ast.Trees.PackageDef
import untpd.{ PackageDef => _, _ }
def extractNewestWrapper(tree: Tree): Name = tree match {
Expand All @@ -216,21 +234,50 @@ class ReplDriver(settings: Array[String],
compiler
.compile(parsed)
.fold(
displayErrors,
{
case (errors: Errors) =>
displayErrors(errors)
},
{
case (unit: CompilationUnit, newState: State) => {
val newestWrapper = extractNewestWrapper(unit.untpdTree)
val newImports = newState.imports ++ extractImports(parsed.trees)(newState.run.runContext)
val newStateWithImports = newState.copy(imports = newImports)

displayDefinitions(unit.tpdTree, newestWrapper)(newStateWithImports)
val (decls, optIndex, symbol) = getDefinitions(unit.tpdTree, newestWrapper)(newStateWithImports)
if (!silent) displayDefinitions(decls, symbol)(newStateWithImports)

optIndex.map(i => newStateWithImports.copy(valIndex = i)).getOrElse(newStateWithImports)
}
}
)
}

/** Display definitions from `tree` */
private def displayDefinitions(tree: tpd.Tree, newestWrapper: Name)(implicit state: State): State = {
/** Display definitions from `symbol` */
private def displayDefinitions(decls: Seq[String], symbol: Option[Symbol])(implicit state: State): Unit = {
implicit val ctx = state.run.runContext

def displayMembers(decls: Seq[String]): Unit = {
decls.foreach(str => out.println(SyntaxHighlighting(str)))
}

def isSyntheticCompanion(sym: Symbol) =
sym.is(Module) && sym.is(Synthetic)
Copy link
Contributor

Choose a reason for hiding this comment

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

sym.is(Module | Synthetic)

Copy link
Author

@3shv 3shv Sep 4, 2017

Choose a reason for hiding this comment

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

Changing sym.is(Module) && sym.is(Synthetic) to sym.is(Module | Synthetic) made some test cases fail. So I didn't use this as of now

Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be the case. Could you try again with sym.is(Module | Synthetic). If that does not work you might have found a bug.

Copy link
Author

Choose a reason for hiding this comment

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

I tried again. This is the error

scala> Test.foo(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109)
val res0: Int = 42
actual ===========>
scala> object Test { def foo(p1: Int, p2: Int, p3: Int, p4: Int, p5: Int, p6: Int, p7: Int, p8: Int, p9: Int, p10: Int, p11: Int, p12: Int, p13: Int, p14: Int, p15: Int, p16: Int, p17: Int, p18: Int, p19: Int, p20: Int, p21: Int, p22: Int, p23: Int, p24: Int, p25: Int, p26: Int, p27: Int, p28: Int, p29: Int, p30: Int, p31: Int, p32: Int, p33: Int, p34: Int, p35: Int, p36: Int, p37: Int, p38: Int, p39: Int, p40: Int, p41: Int, p42: Int, p43: Int, p44: Int, p45: Int, p46: Int, p47: Int, p48: Int, p49: Int, p50: Int, p51: Int, p52: Int, p53: Int, p54: Int, p55: Int, p56: Int, p57: Int, p58: Int, p59: Int, p60: Int, p61: Int, p62: Int, p63: Int, p64: Int, p65: Int, p66: Int, p67: Int, p68: Int, p69: Int, p70: Int, p71: Int, p72: Int, p73: Int, p74: Int, p75: Int, p76: Int, p77: Int, p78: Int, p79: Int, p80: Int, p81: Int, p82: Int, p83: Int, p84: Int, p85: Int, p86: Int, p87: Int, p88: Int, p89: Int, p90: Int, p91: Int, p92: Int, p93: Int, p94: Int, p95: Int, p96: Int, p97: Int, p98: Int, p99: Int, p100: Int, p101: Int, p102: Int, p103: Int, p104: Int, p105: Int, p106: Int, p107: Int, p108: Int, p109: Int): Int = 42 }
scala> Test.foo(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109)
val res0: Int = 42
[error] Test dotty.tools.repl.ScriptedTests.replTests failed: java.lang.AssertionError: Error in file /Users/rveeranki/code/dotty/compiler/target/scala-2.12/test-classes/repl/functions, expected output did not match actual, took 1.259 sec

Basically Object Foo doesn't print anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you're not printing the definition of Foo for some reason, right?

Have a look at: https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/repl/ReplDriver.scala#L226

Copy link
Author

Choose a reason for hiding this comment

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

I even tried on current master by changing sym.is(Module) && sym.is(Synthetic) to sym.is(Module | Synthetic)
Repl goes silent for object Foo however if I change it to sym.is(Module & Synthetic) it prints // defined object foo. So it looks like this behaviour is not introduced by these changes.


def displayTypeDefs(optSymbol: Option[Symbol]): Unit = optSymbol.foreach(sym => sym.info.memberClasses
.collect {
case x if !isSyntheticCompanion(x.symbol) && !x.symbol.name.isReplWrapperName =>
x.symbol
}
.foreach { sym =>
out.println(SyntaxHighlighting("// defined " + sym.showUser))
})
displayTypeDefs(symbol)
displayMembers(decls)
}

/** get definitions from `tree` */
private def getDefinitions(tree: tpd.Tree, newestWrapper: Name)(implicit state: State): (Seq[String], Option[Int], Option[Symbol]) = {
implicit val ctx = state.run.runContext

def resAndUnit(denot: Denotation) = {
Expand All @@ -244,8 +291,7 @@ class ReplDriver(settings: Array[String],
name.startsWith(str.REPL_RES_PREFIX) && hasValidNumber && sym.info == defn.UnitType
}

def displayMembers(symbol: Symbol) = if (tree.symbol.info.exists) {
val info = symbol.info
def getDeclarationsAndIndex(info: Type): (Seq[String], Int) = {
val defs =
info.bounds.hi.finalResultType
.membersBasedOnFlags(Method, Accessor | ParamAccessor | Synthetic | Private)
Expand All @@ -263,54 +309,40 @@ class ReplDriver(settings: Array[String],
val typeAliases =
info.bounds.hi.typeMembers.filter(_.symbol.info.isInstanceOf[TypeAlias])

(
typeAliases.map("// defined alias " + _.symbol.showUser) ++
val declarations = typeAliases.map("// defined alias " + _.symbol.showUser) ++
defs.map(rendering.renderMethod) ++
vals.map(rendering.renderVal).flatten
).foreach(str => out.println(SyntaxHighlighting(str)))

state.copy(valIndex = state.valIndex - vals.filter(resAndUnit).length)
(declarations, state.valIndex - vals.count(resAndUnit))
}
else state

def isSyntheticCompanion(sym: Symbol) =
sym.is(Module) && sym.is(Synthetic)

def displayTypeDefs(sym: Symbol) = sym.info.memberClasses
.collect {
case x if !isSyntheticCompanion(x.symbol) && !x.symbol.name.isReplWrapperName =>
x.symbol
}
.foreach { sym =>
out.println(SyntaxHighlighting("// defined " + sym.showUser))
}


ctx.atPhase(ctx.typerPhase.next) { implicit ctx =>

// Display members of wrapped module:
// get members of wrapped module:
tree.symbol.info.memberClasses
.find(_.symbol.name == newestWrapper.moduleClassName)
.map { wrapperModule =>
displayTypeDefs(wrapperModule.symbol)
displayMembers(wrapperModule.symbol)
if (tree.symbol.info.exists) {
val (decls, index) = getDeclarationsAndIndex(wrapperModule.symbol.info)
(decls, Some(index), Some(wrapperModule.symbol))
} else (Seq.empty, None, None)
}
.getOrElse {
// user defined a trait/class/object, so no module needed
state
(Seq.empty, None, None)
}
}
}

/** Interpret `cmd` to action and propagate potentially new `state` */
private def interpretCommand(cmd: Command)(implicit state: State): State = cmd match {
private def interpretCommand(cmd: Command, silent: Boolean)(implicit state: State): State = cmd match {
case UnknownCommand(cmd) => {
out.println(s"""Unknown command: "$cmd", run ":help" for a list of commands""")
if (!silent) out.println(s"""Unknown command: "$cmd", run ":help" for a list of commands""")
state.withHistory(s"$cmd")
}

case Help => {
out.println(Help.text)
if (!silent) out.println(Help.text)
state.withHistory(Help.command)
}

Expand All @@ -320,7 +352,7 @@ class ReplDriver(settings: Array[String],
}

case Imports => {
state.imports foreach { case (_, i) => println(SyntaxHighlighting(i)) }
if (!silent) state.imports foreach { case (_, i) => println(SyntaxHighlighting(i)) }
state.withHistory(Imports.command)
}

Expand All @@ -330,23 +362,20 @@ class ReplDriver(settings: Array[String],
if (file.exists) {
val contents = scala.io.Source.fromFile(path).mkString
ParseResult(contents)(state.run.runContext) match {
case parsed: Parsed =>
compile(parsed).withHistory(loadCmd)
case SyntaxErrors(_, errors, _) =>
displayErrors(errors).withHistory(loadCmd)
case _ =>
state.withHistory(loadCmd)
case parsed: Parsed => compile(parsed, silent)
case SyntaxErrors(_, errors, _) => displayErrors(errors)
case _ => state
}
}
}.withHistory(loadCmd)
else {
out.println(s"""Couldn't find file "${file.getCanonicalPath}"""")
state.withHistory(loadCmd)
}
state
}.withHistory(loadCmd)

case TypeOf(expr) => {
compiler.typeOf(expr).fold(
displayErrors,
res => out.println(SyntaxHighlighting(res))
res => if (!silent) out.println(SyntaxHighlighting(res))
)
state.withHistory(s"${TypeOf.command} $expr")
}
Expand Down
20 changes: 1 addition & 19 deletions sbt-bridge/src/xsbt/ConsoleInterface.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,6 @@ class ConsoleInterface {
def run(args: Array[String],
bootClasspathString: String,
classpathString: String,
// TODO: initial commands needs to be run under some form of special
// "silent" mode in the REPL. I.e. the effects should be had without
// any visual output.
//
// To do this we can use the `run` interface to the `ReplDriver` and
// pass it a special instance of `ParseResult` like `Silently(res: ParseResult)`
// and then observe the effects without printing to `ReplDriver#out`
//
// This way, the REPL can offer feedback on invalid commands but
// still function without stringly logic.
//
// This same principle can be applied to `cleanupCommands` and
// `bindValues`
//
// Steps:
//
// 1. Introduce `case class Silent(res: ParseResult) extends ParseResult`
// 2. Perform all steps in `interpret` as usual without printing to `out`
initialCommands: String,
cleanupCommands: String,
loader: ClassLoader,
Expand All @@ -51,6 +33,6 @@ class ConsoleInterface {
} ++
Copy link
Contributor

Choose a reason for hiding this comment

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

Try matching the signature here. That might fix your failing tests.

Copy link
Author

Choose a reason for hiding this comment

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

I changed the signature of run to match. Still no luck :(

Array("-classpath", classpathString)

new ReplDriver(completeArgs, classLoader = Some(loader)).runUntilQuit()
new ReplDriver(completeArgs, classLoader = Some(loader), initialCommands = Some(initialCommands), cleanupCommands = Some(cleanupCommands)).runUntilQuit()
}
}