Skip to content
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

Dotty REPL initial version #1082

Merged
merged 18 commits into from
Feb 18, 2016
Merged

Dotty REPL initial version #1082

merged 18 commits into from
Feb 18, 2016

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Feb 14, 2016

An initial version of a REPL for Dotty. Basic functionality works.

Class dotty.tools.dotc.repl.Main is the main entry point. To start the REPL, before we have a dedicated command, use

 java dotty.tools.dotc.repl.Main

(when started under scala, it does not find an Execution context. Hopefully somebody else is better than me at figuring this out).

A comment below explains how the REPL code was derived, and contains a list of TODOs with suggestions to improve it. I am very much hoping for the contributions of others to move this forward.

Review by @hubertp or @DarkDimius or @smarter or anybody wishing to contribute.

@odersky
Copy link
Contributor Author

odersky commented Feb 14, 2016

Explanations, copied from comment on repl.Main.scala:

This REPL was adapted from an old (2008-ish) version of the Scala
REPL. The original version from which the adaptation was done is found in:

 https://github.com/odersky/legacy-svn-scala/tree/spoon

The reason this version was picked instead of a more current one is that
the older version is much smaller, therefore easier to port. It is also
considerably less intertwined with nsc than later versions.

There are a number of TODOs:

  • re-enable jline support (urgent, easy, see TODO in InteractiveReader.scala)
  • figure out why we can launch REPL only with java, not with scala.
  • make a doti command (urgent, easy)
  • create or port REPL tests (urgent, intermediate)
  • make interpreter run a pseudo line on startup to pre-load compiler (somewhat urgent, easy)
  • copy improvements of current Scala REPL wrt to this version
    (somewhat urgent, intermediate)
  • re-enable bindSettings (not urgent, easy, see TODO in InterpreterLoop.scala)
  • make string generation more functional (not urgent, easy)
  • better handling of ^C (not urgent, intermediate)
  • syntax highlighting (not urgent, intermediate)
  • integrate with presentation compiler for command completion (not urgent, hard)

private var binderNum = 0

override def bind(name: String, boundType: String, value: Any)(implicit ctx: Context): Interpreter.Result = {
val binderName = "binder" + binderNum
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe binder$ to ensure absence of name clashes?

@smarter
Copy link
Member

smarter commented Feb 14, 2016

I'd be interested in hearing from @retronym or others familiar with the scalac REPL (@som-snytt ?) about pitfalls we could fall into and mistakes we are likely to repeat. (Suggestions on cool things we could do are also welcome!)

@som-snytt
Copy link
Contributor

I subscribed to this PR this morning because I'm interested in contributing code. I'll make an effort to make time for it.

@VladimirNik
Copy link
Contributor

@odersky To launch repl with scala I used:

scala -J-Xbootclasspath/a:\
[path]/.ivy2/cache/me.d-d/scala-compiler/jars/scala-compiler-2.11.5-[version-info].jar:\
[path-to-dotty]/dotty/bin/../target/scala-2.11/dotty_2.11-0.1-SNAPSHOT.jar \
dotty.tools.dotc.repl.Main

Fork of scala compiler from me.d-d and dotty are added to bootclasspath.

@retronym
Copy link
Member

I'd be interested in hearing from @retronym or others familiar with the scalac REPL

Adding tab completion support to the REPL will be a good way to start experimenting with ways to make the typechecker IDE friendly, without the waiting for integration into Eclipse. This will be handy to validate that the parser and typechecker are fault tolerant and don't discard trees that correspond to the textual sources.

See scala/scala#4766 for an example of such a problem in Scalac, and scala/scala#4725 for the change to use the presentation compiler to drive tab completion.

Try to avoid splicing user-written code into the synthetic wrappers as text. Ideally you want to be able to typecheck the expression as entered after programattically setting up a typechecker context that includes the imports of previously defined symbols and with previously entered imports. Once that is typechecked, it could be spliced into the synthetic wrapper via AST manipulation. This avoids awkward problems of quoting / escaping, and could probably avoid the need to have the deeply nested $iw modules polluting the owner chain of definitions.

@@ -1,20 +0,0 @@
object Test {
Copy link
Member

Choose a reason for hiding this comment

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

Why was this test deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

t920 fails on MacOS. See #1077. I'll push a version in pending.

On Mon, Feb 15, 2016 at 3:01 PM, Guillaume Martres <notifications@github.com

wrote:

In tests/run/t920.scala
#1082 (comment):

@@ -1,20 +0,0 @@
-object Test {

Why was this test deleted?


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

Martin Odersky
EPFL

Copy link
Member

Choose a reason for hiding this comment

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

I'll push a version in pending.

What about just renaming b to something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then I don't know what it would test?

On Mon, Feb 15, 2016 at 5:40 PM, Guillaume Martres <notifications@github.com

wrote:

In tests/run/t920.scala
#1082 (comment):

@@ -1,20 +0,0 @@
-object Test {

I'll push a version in pending.

What about just renaming b to something else?


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

Martin Odersky
EPFL

Copy link
Member

Choose a reason for hiding this comment

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

Not sure either, but I don't think that this test was added to scalac to test for that, cf https://issues.scala-lang.org/browse/SI-920 and scala/scala@485a79a#diff-2a17630cbd90645d1f9a67af6e848c30
If it doesn't test anything useful then we can just drop it.

@odersky
Copy link
Contributor Author

odersky commented Feb 16, 2016

@som-snytt Contributions are very welcome!
@retronym I agree. Widening the interface to the compiler can streamline things a lot and can provide useful new functionality.

@odersky
Copy link
Contributor Author

odersky commented Feb 17, 2016

After discussion at dotty meeting we decided to merge

The interpreter needs to install a virtual directory
as output directory. This is not supported with the -d
option in ScalaSettings. The solution is to make the
output directory overridable in the GenBCode phase.
Allows to replace existing phase by sequence of new
phases.
Adaptation of REPL by Spoon from ca 2007. Compiles OK, but
not yet tested.
Makes side-effecting initialization of interpreter
unnecessary.
Changes necessary to make basic REPL functionality work.

Major refactoing: Code of Interpreter is now in CompilingInterpreter.scala.
Interpreter.scala contains just the API.
When defining a class in the interpreter we had a case where
the class was accessed at phase 46 in the backend, yet the denotation
was the initial denotation in a previous run. In that case we
have to check again at the phase where the denotation is valid.
This was not done before, and hence the owner of the denbotation
did not contain the symbol because the backend phase is after flatten.
It seems some symbols are valid from NoPhase (0). In any case, we should
not check members before typerphase.
Got deleted by accident. Version in run has object
renamed to prevent case clashes on MacOS. Version that
exhibits the clash is in pending/run.
Seems to be overkill for the current interpreter. The only thing that was needed
was a configrable linewidth. A plain setting works fine for this and is in
line with the way things are done elsewhere.
compileString is not needed and does not what one might
expect (no wrapping). So it should not be exported.
DarkDimius added a commit that referenced this pull request Feb 18, 2016
@DarkDimius DarkDimius merged commit 7a893f5 into scala:master Feb 18, 2016
@allanrenucci allanrenucci deleted the add-repl branch December 14, 2017 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants