From fc2717cf44a5e786c89f3586ef6c2b31f6230307 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Wed, 8 Aug 2018 17:59:01 +0200 Subject: [PATCH 1/3] Fix #4230: Handle import in the REPL correctly We used to collect user defined imports from the parsed tree and insert them as untyped trees at the top of the REPL wrapper object. This caused members shadowing issues. We now introduce a phase in the REPL compiler that collects imports after type checking and store them as typed tree. We can then create a context with its imports set in the correct order and use it to compile future expressions. This also fixes #4978 by having user defined import in the run context. Auto-completions somehow ignored them when they were part of the untyped tree. --- compiler/src/dotty/tools/dotc/Compiler.scala | 2 +- compiler/src/dotty/tools/dotc/Run.scala | 2 +- .../tools/repl/CollectTopLevelImports.scala | 30 +++++++ .../src/dotty/tools/repl/ReplCompiler.scala | 87 ++++++++++--------- .../src/dotty/tools/repl/ReplDriver.scala | 28 +++--- .../src/dotty/tools/repl/ReplFrontEnd.scala | 1 - compiler/test-resources/repl/import-shadowing | 27 ++++++ .../dotty/tools/repl/TabcompleteTests.scala | 20 +++++ 8 files changed, 144 insertions(+), 53 deletions(-) create mode 100644 compiler/src/dotty/tools/repl/CollectTopLevelImports.scala create mode 100644 compiler/test-resources/repl/import-shadowing diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index c2e4fa0ad8cb..dafbe2584064 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -111,7 +111,7 @@ class Compiler { List(new Flatten, // Lift all inner classes to package scope new RenameLifted, // Renames lifted classes to local numbering scheme new TransformWildcards, // Replace wildcards with default values - new MoveStatics, // Move static methods to companion classes + new MoveStatics, // Move static methods from companion to the class itself new ExpandPrivate, // Widen private definitions accessed from nested classes new RestoreScopes, // Repair scopes rendered invalid by moving definitions in prior phases of the group new SelectStatic, // get rid of selects that would be compiled into GetStatic diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index d30b2808fcde..52694c0e5662 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -147,7 +147,7 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint compileUnits()(ctx) } - protected def compileUnits()(implicit ctx: Context) = Stats.maybeMonitored { + private def compileUnits()(implicit ctx: Context) = Stats.maybeMonitored { if (!ctx.mode.is(Mode.Interactive)) // IDEs might have multi-threaded access, accesses are synchronized ctx.base.checkSingleThreaded() diff --git a/compiler/src/dotty/tools/repl/CollectTopLevelImports.scala b/compiler/src/dotty/tools/repl/CollectTopLevelImports.scala new file mode 100644 index 000000000000..119810fddc73 --- /dev/null +++ b/compiler/src/dotty/tools/repl/CollectTopLevelImports.scala @@ -0,0 +1,30 @@ +package dotty.tools.repl + +import dotty.tools.dotc.ast.Trees._ +import dotty.tools.dotc.ast.tpd +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.core.Phases.Phase + +/** A phase that collects user defined top level imports. + * + * These imports must be collected as typed trees and therefore + * after Typer. + */ +class CollectTopLevelImports extends Phase { + import tpd._ + + def phaseName = "collectTopLevelImports" + + private[this] var myImports: List[Import] = _ + def imports = myImports + + def run(implicit ctx: Context): Unit = { + def topLevelImports(tree: Tree) = { + val PackageDef(_, _ :: TypeDef(_, rhs: Template) :: Nil) = tree + rhs.body.collect { case tree: Import => tree } + } + + val tree = ctx.compilationUnit.tpdTree + myImports = topLevelImports(tree) + } +} diff --git a/compiler/src/dotty/tools/repl/ReplCompiler.scala b/compiler/src/dotty/tools/repl/ReplCompiler.scala index adbd6419b23e..2a19a6a78c58 100644 --- a/compiler/src/dotty/tools/repl/ReplCompiler.scala +++ b/compiler/src/dotty/tools/repl/ReplCompiler.scala @@ -14,6 +14,7 @@ import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames._ import dotty.tools.dotc.core.Symbols._ import dotty.tools.dotc.reporting.diagnostic.messages +import dotty.tools.dotc.transform.PostTyper import dotty.tools.dotc.typer.{FrontEnd, ImportInfo} import dotty.tools.dotc.util.Positions._ import dotty.tools.dotc.util.SourceFile @@ -23,42 +24,52 @@ import dotty.tools.repl.results._ import scala.collection.mutable -/** This subclass of `Compiler` replaces the appropriate phases in order to - * facilitate the REPL +/** This subclass of `Compiler` is adapted for use in the REPL. * - * Specifically it replaces the front end with `REPLFrontEnd`, and adds a - * custom subclass of `GenBCode`. The custom `GenBCode`, `REPLGenBCode`, works - * in conjunction with a specialized class loader in order to load virtual - * classfiles. + * - compiles parsed expression in the current REPL state: + * - adds the appropriate imports in scope + * - wraps expressions into a dummy object + * - provides utility to query the type of an expression + * - provides utility to query the documentation of an expression */ class ReplCompiler extends Compiler { - override protected def frontendPhases: List[List[Phase]] = - Phases.replace(classOf[FrontEnd], _ => new REPLFrontEnd :: Nil, super.frontendPhases) - def newRun(initCtx: Context, objectIndex: Int) = new Run(this, initCtx) { - override protected[this] def rootContext(implicit ctx: Context) = - addMagicImports(super.rootContext) - - private def addMagicImports(initCtx: Context): Context = { - def addImport(path: TermName)(implicit ctx: Context) = { - val importInfo = ImportInfo.rootImport { () => - ctx.requiredModuleRef(path) - } - ctx.fresh.setNewScope.setImportInfo(importInfo) + override protected def frontendPhases: List[List[Phase]] = List( + List(new REPLFrontEnd), + List(new CollectTopLevelImports), + List(new PostTyper) + ) + + def newRun(initCtx: Context, state: State): Run = new Run(this, initCtx) { + + /** Import previous runs and user defined imports */ + override protected[this] def rootContext(implicit ctx: Context): Context = { + def importContext(imp: tpd.Import)(implicit ctx: Context) = + ctx.importContext(imp, imp.symbol) + + def importPreviousRun(id: Int)(implicit ctx: Context) = { + // we first import the wrapper object id + val path = nme.EMPTY_PACKAGE ++ "." ++ objectNames(id) + val importInfo = ImportInfo.rootImport(() => + ctx.requiredModuleRef(path)) + val ctx0 = ctx.fresh.setNewScope.setImportInfo(importInfo) + + // then its user defined imports + val imports = state.imports.getOrElse(id, Nil) + if (imports.isEmpty) ctx0 + else imports.foldLeft(ctx0.fresh.setNewScope)((ctx, imp) => + importContext(imp)(ctx)) } - (1 to objectIndex) - .foldLeft(initCtx) { (ictx, i) => - addImport(nme.EMPTY_PACKAGE ++ "." ++ objectNames(i))(ictx) - } + (1 to state.objectIndex).foldLeft(super.rootContext)((ctx, id) => + importPreviousRun(id)(ctx)) } } private[this] val objectNames = mutable.Map.empty[Int, TermName] private def objectName(state: State) = - objectNames.getOrElseUpdate(state.objectIndex, { - (str.REPL_SESSION_LINE + state.objectIndex).toTermName - }) + objectNames.getOrElseUpdate(state.objectIndex, + (str.REPL_SESSION_LINE + state.objectIndex).toTermName) private case class Definitions(stats: List[untpd.Tree], state: State) @@ -86,7 +97,7 @@ class ReplCompiler extends Compiler { } Definitions( - state.imports ++ defs, + defs, state.copy( objectIndex = state.objectIndex + (if (defs.isEmpty) 0 else 1), valIndex = valIdx @@ -158,19 +169,18 @@ class ReplCompiler extends Compiler { def docOf(expr: String)(implicit state: State): Result[String] = { implicit val ctx: Context = state.context - /** - * Extract the "selected" symbol from `tree`. + /** Extract the "selected" symbol from `tree`. * - * Because the REPL typechecks an expression, special syntax is needed to get the documentation - * of certain symbols: + * Because the REPL typechecks an expression, special syntax is needed to get the documentation + * of certain symbols: * - * - To select the documentation of classes, the user needs to pass a call to the class' constructor - * (e.g. `new Foo` to select `class Foo`) - * - When methods are overloaded, the user needs to enter a lambda to specify which functions he wants - * (e.g. `foo(_: Int)` to select `def foo(x: Int)` instead of `def foo(x: String)` + * - To select the documentation of classes, the user needs to pass a call to the class' constructor + * (e.g. `new Foo` to select `class Foo`) + * - When methods are overloaded, the user needs to enter a lambda to specify which functions he wants + * (e.g. `foo(_: Int)` to select `def foo(x: Int)` instead of `def foo(x: String)` * - * This function returns the right symbol for the received expression, and all the symbols that are - * overridden. + * This function returns the right symbol for the received expression, and all the symbols that are + * overridden. */ def extractSymbols(tree: tpd.Tree): Iterator[Symbol] = { val sym = tree match { @@ -210,7 +220,7 @@ class ReplCompiler extends Compiler { import untpd._ val valdef = ValDef("expr".toTermName, TypeTree(), Block(trees, unitLiteral)) - val tmpl = Template(emptyConstructor, Nil, EmptyValDef, state.imports :+ valdef) + val tmpl = Template(emptyConstructor, Nil, EmptyValDef, List(valdef)) val wrapper = TypeDef("$wrapper".toTypeName, tmpl) .withMods(Modifiers(Final)) .withPos(Position(0, expr.length)) @@ -261,9 +271,8 @@ class ReplCompiler extends Compiler { if (errorsAllowed || !ctx.reporter.hasErrors) unwrapped(unit.tpdTree, src) - else { + else ctx.reporter.removeBufferedMessages.errors - } } } } diff --git a/compiler/src/dotty/tools/repl/ReplDriver.scala b/compiler/src/dotty/tools/repl/ReplDriver.scala index 59732aa959b1..47e77062bda6 100644 --- a/compiler/src/dotty/tools/repl/ReplDriver.scala +++ b/compiler/src/dotty/tools/repl/ReplDriver.scala @@ -44,12 +44,12 @@ import scala.collection.JavaConverters._ * * @param objectIndex the index of the next wrapper * @param valIndex the index of next value binding for free expressions - * @param imports the list of user defined imports + * @param imports a map from object index to the list of user defined imports * @param context the latest compiler context */ case class State(objectIndex: Int, valIndex: Int, - imports: List[untpd.Import], + imports: Map[Int, List[tpd.Import]], context: Context) /** Main REPL instance, orchestrating input, compilation and presentation */ @@ -64,14 +64,14 @@ class ReplDriver(settings: Array[String], /** Create a fresh and initialized context with IDE mode enabled */ private[this] def initialCtx = { - val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions).addMode(Mode.Interactive).addMode(Mode.ReadComments) + val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions | Mode.Interactive | Mode.ReadComments) val ictx = setup(settings, rootCtx)._2 ictx.base.initialize()(ictx) ictx } /** the initial, empty state of the REPL session */ - final def initialState = State(0, 0, Nil, rootCtx) + final def initialState = State(0, 0, Map.empty, rootCtx) /** Reset state of repl to the initial state * @@ -144,7 +144,7 @@ class ReplDriver(settings: Array[String], Console.withOut(out) { Console.withErr(out) { op } } private def newRun(state: State) = { - val run = compiler.newRun(rootCtx.fresh.setReporter(newStoreReporter), state.objectIndex) + val run = compiler.newRun(rootCtx.fresh.setReporter(newStoreReporter), state) state.copy(context = run.runContext) } @@ -177,9 +177,6 @@ class ReplDriver(settings: Array[String], .getOrElse(Nil) } - private def extractImports(trees: List[untpd.Tree]): List[untpd.Import] = - trees.collect { case imp: untpd.Import => imp } - private def interpret(res: ParseResult)(implicit state: State): State = { val newState = res match { case parsed: Parsed if parsed.trees.nonEmpty => @@ -209,6 +206,9 @@ class ReplDriver(settings: Array[String], case _ => nme.NO_NAME } + def extractTopLevelImports(ctx: Context): List[tpd.Import] = + ctx.phases.collectFirst { case phase: CollectTopLevelImports => phase.imports }.get + implicit val state = newRun(istate) compiler .compile(parsed) @@ -217,8 +217,11 @@ class ReplDriver(settings: Array[String], { case (unit: CompilationUnit, newState: State) => val newestWrapper = extractNewestWrapper(unit.untpdTree) - val newImports = newState.imports ++ extractImports(parsed.trees) - val newStateWithImports = newState.copy(imports = newImports) + val newImports = extractTopLevelImports(newState.context) + var allImports = newState.imports + if (newImports.nonEmpty) + allImports += (newState.objectIndex -> newImports) + val newStateWithImports = newState.copy(imports = allImports) val warnings = newState.context.reporter.removeBufferedMessages(newState.context) displayErrors(warnings)(newState) // display warnings @@ -315,7 +318,10 @@ class ReplDriver(settings: Array[String], initialState case Imports => - state.imports.foreach(i => out.println(SyntaxHighlighting(i.show(state.context)))) + for { + objectIndex <- 1 to state.objectIndex + imp <- state.imports.getOrElse(objectIndex, Nil) + } out.println(imp.show(state.context)) state case Load(path) => diff --git a/compiler/src/dotty/tools/repl/ReplFrontEnd.scala b/compiler/src/dotty/tools/repl/ReplFrontEnd.scala index 9b10bfa9b414..7f6091729f1c 100644 --- a/compiler/src/dotty/tools/repl/ReplFrontEnd.scala +++ b/compiler/src/dotty/tools/repl/ReplFrontEnd.scala @@ -12,7 +12,6 @@ import dotc.core.Contexts.Context * compiler pipeline. */ private[repl] class REPLFrontEnd extends FrontEnd { - override def phaseName = "frontend" override def isRunnable(implicit ctx: Context) = true diff --git a/compiler/test-resources/repl/import-shadowing b/compiler/test-resources/repl/import-shadowing new file mode 100644 index 000000000000..71dc48dbe3c2 --- /dev/null +++ b/compiler/test-resources/repl/import-shadowing @@ -0,0 +1,27 @@ +scala> object A { def f = 1 } +// defined object A + +scala> object B { def f = 2 } +// defined object B + +scala> import A._ + +scala> val x0 = f +val x0: Int = 1 + +scala> import B._ + +scala> val x1 = f +val x1: Int = 2 + +scala> def f = 3 +def f: Int + +scala> val x2 = f +val x2: Int = 3 + +scala> def f = 4; import A._ +def f: Int + +scala> val x3 = f +val x3: Int = 1 diff --git a/compiler/test/dotty/tools/repl/TabcompleteTests.scala b/compiler/test/dotty/tools/repl/TabcompleteTests.scala index c5c84c7b88bc..75f28c15a213 100644 --- a/compiler/test/dotty/tools/repl/TabcompleteTests.scala +++ b/compiler/test/dotty/tools/repl/TabcompleteTests.scala @@ -60,4 +60,24 @@ class TabcompleteTests extends ReplTest { val expected = List("comp1", "comp2", "comp3") assertEquals(expected, tabComplete("(new Foo).comp").sorted) } + + @Test def completeFromPreviousState2 = + fromInitialState { implicit state => + val src = "def hello = 1" + run(src) + } + .andThen { implicit state => + val expected = List("hello") + assertEquals(expected, tabComplete("hel")) + } + + @Test def tabCompleteFromPreviousImport = + fromInitialState { implicit state => + val src = "import java.io.FileDescriptor" + run(src) + } + .andThen { implicit state => + val expected = List("FileDescriptor") + assertEquals(expected, tabComplete("val foo: FileDesc")) + } } From bb69949f217b6de6ebe5fa0fe00133d8b1169925 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Wed, 22 Aug 2018 16:19:18 +0200 Subject: [PATCH 2/3] Workaround #4988 --- compiler/src/dotty/tools/repl/ReplCompiler.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/repl/ReplCompiler.scala b/compiler/src/dotty/tools/repl/ReplCompiler.scala index 2a19a6a78c58..6a3fb7ebd4a2 100644 --- a/compiler/src/dotty/tools/repl/ReplCompiler.scala +++ b/compiler/src/dotty/tools/repl/ReplCompiler.scala @@ -272,7 +272,7 @@ class ReplCompiler extends Compiler { if (errorsAllowed || !ctx.reporter.hasErrors) unwrapped(unit.tpdTree, src) else - ctx.reporter.removeBufferedMessages.errors + ctx.reporter.removeBufferedMessages.errors[tpd.ValDef] // Workaround #4988 } } } From 52a604c604f502c8934deeb4bfd41c76d418d389 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Wed, 22 Aug 2018 16:20:22 +0200 Subject: [PATCH 3/3] Add missing `stripMargin` --- compiler/src/dotty/tools/dotc/transform/Pickler.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 46e437938cc8..9a98ae24921b 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -112,8 +112,8 @@ class Pickler extends Phase { if (previous != unpickled) { output("before-pickling.txt", previous) output("after-pickling.txt", unpickled) - ctx.error(s"""pickling difference for ${cls} in ${cls.sourceFile}, for details: + ctx.error(s"""pickling difference for $cls in ${cls.sourceFile}, for details: | - | diff before-pickling.txt after-pickling.txt""") + | diff before-pickling.txt after-pickling.txt""".stripMargin) } }