Skip to content

Forward-port REPL improvements #1087

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
odersky opened this issue Feb 14, 2016 · 6 comments
Closed

Forward-port REPL improvements #1087

odersky opened this issue Feb 14, 2016 · 6 comments

Comments

@odersky
Copy link
Contributor

odersky commented Feb 14, 2016

The dotty REPL was derived from a version of the Scala REPL from 2008. Since then there have been many improvements. A lot of them are specific to nsc (e.g. power mode) so are probably difficult to integrate as is. But there are also a lot of other improvements that would make sense to integrate.

I suggest to go back into the history of the Scala REPL and see which commits warrant porting into the new one.

Independent of this, there are also a number of refactorings of the REPL code which would make sense. First, we should make use of string interpolation to construct all synthetic sources functionally instead of using string printers as is now the case. Second, we could possibly get interpreter information directly from typed trees by using a slightly different way to access the compiler.

Long term: An integration of the REPL with a presentation compiler (which remains to be created).

@som-snytt
Copy link
Contributor

I'll offer a repl that supplies a scripter phase (#1085) with tree-directed transforms instead of textual; and also uses retronym's idea of smart imports from contexts to alleviate template wrapping issues.

With a scripter phase that is easy to swap out, issues like garbage collecting objects in history are easier to address with custom scripting. Similarly, customizing a repl as for spark.

@odersky
Copy link
Contributor Author

odersky commented Mar 8, 2016

@som-snytt I think that would be a great contribution. I am particularly keen in the smart imports idea. That should be easy to implement and avpid a lot of complexity.

@retronym
Copy link
Member

retronym commented Mar 8, 2016

Here's a sketch to show how simple the smart import implementation might be (even in Scala-classic!):

diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
index 3e60ef3..a7796c7 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
@@ -224,7 +224,8 @@ trait Contexts { self: Analyzer =>
     protected def outerDepth = if (outerIsNoContext) 0 else outer.depth

     val depth: Int = {
-      val increasesDepth = isRootImport || outerIsNoContext || (outer.scope != scope)
+      def isInRead = outer.enclClassOrMethod.owner.name == TypeName("$read")
+      val increasesDepth = isRootImport || outerIsNoContext || (outer.scope != scope) || isInRead
       ( if (increasesDepth) 1 else 0 ) + outerDepth
     }

diff --git a/src/repl/scala/tools/nsc/interpreter/Imports.scala b/src/repl/scala/tools/nsc/interpreter/Imports.scala
index 6a75432..5246643 100644
--- a/src/repl/scala/tools/nsc/interpreter/Imports.scala
+++ b/src/repl/scala/tools/nsc/interpreter/Imports.scala
@@ -138,10 +138,6 @@ trait Imports {

     // add code for a new object to hold some imports
     def addWrapper() {
-      import nme.{ INTERPRETER_IMPORT_WRAPPER => iw }
-      code append (wrapper.prewrap format iw)
-      trailingBraces append wrapper.postwrap
-      accessPath append s".$iw"
       currentImps.clear()
     }

We already do the same thing for root imports:

import scala._
import java.lang._
import Predef._

is taken to mean:

import scala._
{
  import java.lang._
  {
    import Predef._
    {

We'd also need to tweak a special case exception in the "imported members if permanently hidden by member" warning.

scala> import Predef.toString
[[syntax trees at end of                    parser]] // <console>
package $line16 {
  object $read extends scala.AnyRef {
    def <init>() = {
      super.<init>();
      ()
    };
    import $line4.$read.X;
    import $line4.$read.Y;
    import Y.foo;
    import X.foo;
    import Predef.toString
  }
}

<console>:13: warning: imported `toString` is permanently hidden by definition of method toString in class Object
       import Predef.toString
                     ^

@som-snytt
Copy link
Contributor

@retronym You could have just mailed me the napkin.

@som-snytt
Copy link
Contributor

Times flies when your day job gets busy. I'm working on this in occasional hours. Sorry for the delay.

@smarter
Copy link
Member

smarter commented Jan 11, 2018

Closing, Dotty has a shiny new REPL since #2991

@smarter smarter closed this as completed Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants