Skip to content

Fix #3305 and #3309 #3314

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 3 commits into from
Oct 13, 2017
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
11 changes: 9 additions & 2 deletions compiler/src/dotty/tools/repl/ParseResult.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package repl
import dotc.reporting.diagnostic.MessageContainer
import dotc.core.Contexts.Context
import dotc.parsing.Parsers.Parser
import dotc.parsing.Tokens
import dotc.util.SourceFile
import dotc.ast.untpd
import dotc.reporting._
Expand Down Expand Up @@ -97,6 +98,12 @@ object ParseResult {

@sharable private[this] val CommandExtract = """(:[\S]+)\s*(.*)""".r

private def parseStats(parser: Parser): List[untpd.Tree] = {
val stats = parser.blockStatSeq()
parser.accept(Tokens.EOF)
stats
}

/** Extract a `ParseResult` from the string `sourceCode` */
def apply(sourceCode: String)(implicit ctx: Context): ParseResult =
sourceCode match {
Expand All @@ -114,7 +121,7 @@ object ParseResult {
val source = new SourceFile("<console>", sourceCode.toCharArray)
val parser = new Parser(source)

val (_, stats) = parser.templateStatSeq()
val stats = parseStats(parser)

if (ctx.reporter.hasErrors) {
SyntaxErrors(sourceCode,
Expand All @@ -140,7 +147,7 @@ object ParseResult {
reporter.withIncompleteHandler(_ => _ => needsMore = true) {
val source = new SourceFile("<console>", sourceCode.toCharArray)
val parser = new Parser(source)(ctx.fresh.setReporter(reporter))
parser.templateStatSeq()
parseStats(parser)
!reporter.hasErrors && needsMore
}
}
Expand Down
28 changes: 23 additions & 5 deletions compiler/src/dotty/tools/repl/Rendering.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package dotty.tools
package repl

import java.lang.ClassLoader
import java.io.{ StringWriter, PrintWriter }
import java.lang.{ ClassLoader, ExceptionInInitializerError }
import java.lang.reflect.InvocationTargetException

import scala.util.control.NonFatal

Expand Down Expand Up @@ -70,10 +72,26 @@ private[repl] class Rendering(compiler: ReplCompiler,
/** Render value definition result */
def renderVal(d: Denotation)(implicit ctx: Context): Option[String] = {
val dcl = d.symbol.showUser
val resultValue =
if (d.symbol.is(Flags.Lazy)) Some("<lazy>")
else valueOf(d.symbol)

resultValue.map(value => s"$dcl = $value")
try {
val resultValue =
if (d.symbol.is(Flags.Lazy)) Some("<lazy>")
else valueOf(d.symbol)

resultValue.map(value => s"$dcl = $value")
}
catch { case ex: InvocationTargetException => Some(renderError(ex)) }
}

/** Render the stack trace of the underlying exception */
private def renderError(ex: InvocationTargetException): String = {
val cause = ex.getCause match {
case ex: ExceptionInInitializerError => ex.getCause
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am trying to isolate the underlying exception. This is based on nothing but experiment:

> null.toString
Exception in thread "main" java.lang.reflect.InvocationTargetException
	...
Caused by: java.lang.ExceptionInInitializerError
	...
Caused by: java.lang.NullPointerException
	...
scala> def foo: Int = foo + 1 
def foo: Int
scala> foo 
Exception in thread "main" java.lang.reflect.InvocationTargetException
	...
Caused by: java.lang.StackOverflowError
	...

I am sure there is a better a way to do it.

Copy link
Member

Choose a reason for hiding this comment

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

We really need tests for all of those :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Working on it

case ex => ex
}
val sw = new StringWriter()
val pw = new PrintWriter(sw)
cause.printStackTrace(pw)
sw.toString
}
}
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/repl/ReplCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class ReplCompiler(val directory: AbstractFile) extends Compiler {
* }
* ```
*/
def wrapped(defs: Definitions): untpd.PackageDef = {
def wrapped(defs: Definitions, sourceCode: String): untpd.PackageDef = {
import untpd._

implicit val ctx: Context = defs.state.run.runContext
Expand All @@ -156,7 +156,7 @@ class ReplCompiler(val directory: AbstractFile) extends Compiler {
List(
ModuleDef(objectName(defs.state), tmpl)
.withMods(new Modifiers(Module | Final))
.withPos(Position(defs.stats.head.pos.start, defs.stats.last.pos.end))
.withPos(Position(0, sourceCode.length))
)
}

Expand All @@ -170,7 +170,7 @@ class ReplCompiler(val directory: AbstractFile) extends Compiler {

def createUnit(defs: Definitions, sourceCode: String): Result[CompilationUnit] = {
val unit = new CompilationUnit(new SourceFile(objectName(defs.state).toString, sourceCode))
unit.untpdTree = wrapped(defs)
unit.untpdTree = wrapped(defs, sourceCode)
unit.result
}

Expand Down Expand Up @@ -238,7 +238,7 @@ class ReplCompiler(val directory: AbstractFile) extends Compiler {
PackageDef(Ident(nme.EMPTY_PACKAGE),
TypeDef("EvaluateExpr".toTypeName, tmpl)
.withMods(new Modifiers(Final))
.withPos(Position(trees.head.pos.start, trees.last.pos.end)) :: Nil
.withPos(Position(0, expr.length)) :: Nil
)
}

Expand Down
10 changes: 7 additions & 3 deletions compiler/src/dotty/tools/repl/ReplDriver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ class ReplDriver(settings: Array[String],

private def interpret(res: ParseResult)(implicit state: State): State =
res match {
case parsed: Parsed =>
case parsed: Parsed if parsed.trees.nonEmpty =>
compile(parsed)
.withHistory(parsed.sourceCode :: state.history)
.newRun(compiler, rootCtx)
Expand All @@ -195,9 +195,13 @@ class ReplDriver(settings: Array[String],
displayErrors(errs)
state.withHistory(src :: state.history)

case Newline | SigKill => state

case cmd: Command => interpretCommand(cmd)

case SigKill => // TODO
state

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

/** Compile `parsed` trees and evolve `state` in accordance */
Expand Down
14 changes: 14 additions & 0 deletions compiler/test-resources/repl/parsing
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
scala> ;
scala> ;;
scala> 1; 2
val res0: Int = 1
val res1: Int = 2
scala> 1;
val res2: Int = 1
scala> 1;; 2
val res3: Int = 1
val res4: Int = 2
scala> }
1 | }
| ^
| eof expected, but '}' found
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@felixmulder Why do I need a space after |, even though when I run the repl on my machine it doesn't print the space. Is it a bug in the testing framework?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like it!

23 changes: 23 additions & 0 deletions compiler/test/dotty/tools/repl/CompilerTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,27 @@ class ReplCompilerTests extends ReplTest {
storedOutput()
)
}

@Test def i3305: Unit = {
fromInitialState { implicit s =>
compile("null.toString")
storedOutput().startsWith("java.lang.NullPointerException")
}

fromInitialState { implicit s =>
compile("def foo: Int = 1 + foo; foo")
storedOutput().startsWith("def foo: Int\njava.lang.StackOverflowError")
}

fromInitialState { implicit s =>
compile("""throw new IllegalArgumentException("Hello")""")
storedOutput().startsWith("java.lang.IllegalArgumentException: Hello")
}

// FIXME
// fromInitialState { implicit s =>
// compile("val (x, y) = null")
// storedOutput().startsWith("scala.MatchError: null")
// }
}
}
4 changes: 2 additions & 2 deletions compiler/test/dotty/tools/repl/ScriptedTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import scala.io.Source

import dotc.reporting.MessageRendering

/** Runs all tests contained in `/repl/test-resources/scripts` */
/** Runs all tests contained in `compiler/test-resources/repl/` */
class ScriptedTests extends ReplTest with MessageRendering {

private def scripts(path: String): Array[JFile] = {
Expand Down Expand Up @@ -44,7 +44,7 @@ class ScriptedTests extends ReplTest with MessageRendering {
def evaluate(state: State, input: String, prompt: String) =
try {
val nstate = run(input.drop(prompt.length))(state)
val out = input + "\n" + stripColor(storedOutput())
val out = input + "\n" + storedOutput()
(out, nstate)
}
catch {
Expand Down
6 changes: 6 additions & 0 deletions compiler/test/dotty/tools/repl/TabcompleteTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,10 @@ class TabcompleteTests extends ReplTest {
assert(tabComplete(src2).suggestions.nonEmpty)
}
}

@Test def i3309: Unit =
fromInitialState { implicit s =>
List("\"", "#", ")", "=", "'", "¨", "£", ".", ":", ",", ";", "@", "}", "[", "]")
.foreach(src => assertTrue(tabComplete(src).suggestions.isEmpty))
}
}