Content-Length: 644067 | pFad | https://github.com/scala/scala/pull/4725

02 Use presentation compiler for REPL tab completion by retronym · Pull Request #4725 · scala/scala · GitHub
Skip to content

Use presentation compiler for REPL tab completion #4725

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 23 commits into from
Sep 21, 2015

Conversation

retronym
Copy link
Member

@retronym retronym commented Sep 2, 2015

This brings tab-completion in the REPL in line with the experience in IDEs and ENSIME-powered editors.

  • Reliable completion, also in partial expressions and syntactically incorrect programs: try class C { def f(l: List[Int]) = l.<TAB>
  • CamelCase completion: try (l: List[Int]).rro<TAB>, it expands to (l: List[Int]).reduceRightOption
  • Find members by typing any CamelCased part of the name: try classOf[String].typ<TAB> to get getAnnotationsByType, getComponentType and others
  • Complete non-qualified names, including types: try def f(s: Str<TAB>
  • Show desugarings performed by the compiler by adding //print: try for (x <- 1 to 10) println(x) //print<TAB>
  • Press tab twice to see the method signature: try List(1,2,3).part<TAB>, which completes to List(1,2,3).partition; press tab again to display def partition(p: Int => Boolean): (List[Int], List[Int])
  • It even completes bean getters without typing get: try (d: java.util.Date).day<TAB>!

Review by @som-snytt

retronym and others added 9 commits September 1, 2015 18:13
Fix cross talk between candidates in ImplicitComputation#findAll.

Haoyi reported that he couldn't get the presentation compiler
to offer completion suggestions for extension methods on primitive
arrays.

Turns out that an error in typechecking the previous candidate
had not been cleared, and this was taken to mean that `arrayIntOps`
was itself ineligible.

After this patch:
```
scala> Array(1, 2, 3) reverse
reverseIterator   reverse   reverseMap   reversed

scala> Array(1,2,3).reverse //print
   scala.Predef.intArrayOps(scala.Array.apply(1, 2, 3)).reverse

scala> Array(1, 2, 3) to
toString   toCollection   toSeq      toIterator   to              toMap      toSet          toList
toArray    toBuffer       toStream   toIterable   toTraversable   toVector   toIndexedSeq

scala> Array(1, 2, 3) toSeq
   override def toSeq: Seq[Int]
```
The classic banner is available under -Dscala.repl.power.banner=classic.

```
scala> :power
Power mode enabled. :phase is at typer.
import scala.tools.nsc._, intp.global._, definitions._
Try :help or completions for vals._ and power._
```
Transcript paste mode invites the user to keep typing like
regular paste mode, but really you must enter more transcript.

This matters if the script ends in the middle of incomplete
code that the user wants to complete by hand.

Previously,

```
scala> scala> def f() = {

// Detected repl transcript paste: ctrl-D to finish.

// Replaying 1 commands from transcript.

scala> def f() = {

scala> scala> def f() = {

// Detected repl transcript paste: ctrl-D to finish.

     | }
// Replaying 1 commands from transcript.

scala> def f() = {
}
f: ()Unit
```

Now,

```
scala> scala> def f() = {

// Detected repl transcript. Paste more, or ctrl-D to finish.

// Replaying 1 commands from transcript.

scala> def f() = {
     | 42
     | }
f: ()Int

scala> f()
res0: Int = 42
```
The presentation compiler currently demands that all interaction
is performed by asynchronous submission of work items, which are
queued and executed on the presentation compiler thread.

This is fairly inconvenient if you are a known-single-threaded client
that is trying to use the compiler from your own thread.

This commit adds an option to disable "assertCorrectThread" to better
support this use case.
Conveniences added:

  - The client need not determine ahead of time whether it wants
    scope completions of type member completions, this is inferred
    from the tree at the cursor
  - Computes the delta from the cursor to point where a suggested
    completion item should be written. This includes finding the
    position of the name within a Select node, which is tricky.
  - Includes a matcher that matches results base on prefix,
    accessibility and suitability in the syntactic location.
    Type members are not offered in term positions, and term members
    only offered in type position if they could for a prefix for
    a type. E.g. `type T = glob<TAB>` should offer `global` to help
    writing `global.Tree`.
In preparation for use of the presentation compiler to drive
REPL tab completion, add a dependency in the build.

As far as the outside world is concerned, these are already comingled
in scala-compiler.jar, and this patch doesn't change that.
val and var members of classes are split into getter/setter and field
symbols. However, we can't call `defString` on any of these and see
what existed in source code.

Example:

```
scala> class X { protected var x: Int = 0 }
defined class X

scala> val xField = typeOf[X].member(TermName("x "))
xField: $r.intp.global.Symbol = variable x

scala> xField.defString
res10: String = private[this] var x: Int

scala> xField.getterIn(xField.owner).defString
res11: String = protected def x: Int

scala> xField.setterIn(xField.owner).defString
res12: String = protected def x_=(x$1: Int): Unit
```

This commit introduces a new method on `Symbol` that pieces together
the information from these symbols to create an artificial symbol
that has the `defString` we're after:

```
scala> xField.sugaredSymbolOrSelf.defString
res14: String = protected var x: Int
```
The old implementation is still avaiable under a flag, but we'll
remove it in due course.

Design goal:

  - Push as much code in src/interactive as possible to enable reuse
    outside of the REPL
  - Don't entangle the REPL completion with JLine. The enclosed test
    case drives the REPL and autocompletion programatically.
  - Don't hard code UI choices, like how to render symbols or
    how to filter candidates.

When completion is requested, we wrap the entered code into the
same "interpreter wrapper" synthetic code as is done for regular
execution. We then start a throwaway instance of the presentation
compiler, which takes this as its one and only source file, and
has a classpath formed from the REPL's classpath and the REPL's
output directory (by default, this is in memory).

We can then typecheck the tree, and find the position in the synthetic
source corresponding to the cursor location. This is enough to use
the new completion APIs in the presentation compiler to prepare
a list of candidates.

We go to extra lengths to allow completion of partially typed
identifiers that appear to be keywords, e.g `global.def` should offer
`definitions`.

Two secret handshakes are included; move the the end of the line,
type `// print<TAB>` and you'll see the post-typer tree.
`// typeAt 4 6<TAB>` shows the type of the range position within
the buffer.

The enclosed unit test exercises most of the new functionality.
@scala-jenkins scala-jenkins added this to the 2.11.8 milestone Sep 2, 2015
@retronym
Copy link
Member Author

retronym commented Sep 2, 2015

These commits, slated for 2.12, remove the old implementation and experiment with colorized completion suggestions: f279894...d05b097

We should also upgrade JLIne 2.13.1 when it is ready (scala/scala-dev#44)

@retronym
Copy link
Member Author

retronym commented Sep 2, 2015

Looks like I've missed the any2stringAdd implicit shadowing commit in my rebase. Mabye that can come in a separate PR, assuming it isn't dependent on these changes.

@som-snytt
Copy link
Contributor

Sorry I've been squeezed for time by life + work, the classic rock + hard place respectively.

I think I included that SI-1931 commit b/c what it touches and likely conflicts, ugh. Including test output. I tried & failed one day to normalize the line numbering; I didn't try hard.

I see some of the previous commits were squashed? I'm not partial to history that way, but hard to track at a distance. You've got to raise the barrel a bit and account for wind, then maybe you hit the target? etc.

Soon we can use "resugar" without quotes. I mean resugar.

I hope someday you can comment on an ammonite issue, "fixed in scala repl." Which I guess needs a name now, someone should create an issue.

// Needed to import `xxx` during line 2 of:
// scala> val xxx = ""
// scala> def foo: x<TAB>
importVars += stripped.toTermName
Copy link
Member

Choose a reason for hiding this comment

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

Indent?

Copy link
Contributor

Choose a reason for hiding this comment

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

The comment has to precede the if statement. I noticed it, too, but hoped the typesetter would catch it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed. The last two lines were actually not needed.

Move misplaced and malindented comment and remove two lines of unneeded code.
@retronym
Copy link
Member Author

retronym commented Sep 2, 2015

@som-snytt I usually err on the side of submitting a history of commits that tells the story sub-feature-wise. In this case, given that the actual history was so meandering, and that all of the code here is new, I was a bit more squashy than usual.

@retronym
Copy link
Member Author

retronym commented Sep 2, 2015

Oh, and I'm pretty sure that ammonite (and Scala IDE) will need to play catch up with global.def<TAB>.

@retronym
Copy link
Member Author

retronym commented Sep 3, 2015

Added a few commits as a result of @SethTisue's feedback; results are sorted, and we hide inherited members of the interpreter wrappers.

This makes life easier for clients of these APIs, we use this
to avoid passing this around in the wrapper result `TypeMembers`.
Trying harder to keep the synthetic interpretter wrapper classes
behind the curtain
@retronym retronym force-pushed the topic/completely-2.11 branch from 1d13b9a to 3f1352f Compare September 3, 2015 04:10
@retronym
Copy link
Member Author

retronym commented Sep 3, 2015

I'm not going to include this in this PR, but I couldn't resist trying out some of this hole driven programming that the cool kids are raving about:

https://github.com/retronym/scala/compare/topic/completely-2.11...retronym:topic/completely-expected-type?expand=1

scala> class A; object A extends A; def foo(a: A) = 0; foo(__
   expected type: A

   ???   A   error

scala> class A; object A extends A; def foo(a: Int) = 0; foo(__)
   expected type: Int

   ##   ???   Integer2int   error   foo   hashCode   readInt

@retronym
Copy link
Member Author

retronym commented Sep 3, 2015

Another teaser: we can hijack keybindings in jline.InteractiveReader as follows:

        val CTRL_Q = 17.toChar
        this.getKeys.bind(CTRL_Q.toString, new ActionListener {
          override def actionPerformed(e: ActionEvent): Unit = {
            println("CTRL-Q")
            redrawLineAndFlush()
          }
        })

Could use this for a more convenient version of //print, or to display type errors before submitting the current line, or ...

This is just too useful to leave on the cutting room floor.

```
scala> classOf[String].enclo<TAB>

scala> classOf[String].getEnclosing
getEnclosingClass   getEnclosingConstructor   getEnclosingMethod

scala> classOf[String].simpl<TAB>

scala> classOf[String].getSimpleName

type X = global.TTWD<TAB>

scala> type X = global.TypeTreeWithDeferredRefCheck
```

I revised the API of `matchingResults` as it was clunky to reuse
the filtering on accessibility and term/type-ness while providing
a custom name matcher.
@retronym
Copy link
Member Author

retronym commented Sep 8, 2015

I've added a commit with a cleaned up version of the camel case completion.

@SethTisue
Copy link
Member

/rebuild

@SethTisue
Copy link
Member

the camel case stuff is great, and worked on everything I tried it on

@som-snytt
Copy link
Contributor

This isn't your fault, but right arrow sorts to the bottom. VBAR too. As opposed to category symbols coming first. Because we love symbols and encourage their use.

OK, I see how the getCamel thing works, neat.

I kind of think this should work: classOf[String].typename[\t]. Another common one would be anything with classpath in it.

Basically, all my woes are due to caps; I was going to add ignorecase eventually. I have one machine where it's enabled in shell, another not because I'm lazy to look up the syntax, so I suffer. Yes, I suffer, but I appreciate it all the more when it works.

So I'd want not only typ to complete b/c getTyp, but also all because it's on a camel boundary in notifyAll.

I guess we'd want mous to complete to getMouse but not isAnonymous?

You probably noticed it offers <byname> et al.

I don't suppose you'll be offering XML completion? :load mydoc.dtd Just kidding.

The new name for Typesafe could be 403 possibilities.

scala> Display all 403 possibilities? (y or n)

It's biased to beans: inst -> isInstanceOf despite asInstanceOf.

More reasonably buffered trumps toBuffer on iterator, or indexOf/Where for toIndexedSeq.

But should to completes include copyToArray? padTo?

@retronym
Copy link
Member Author

I've dropped the classOf changes for now and tracked them as https://issues.scala-lang.org/browse/SI-9477.

@lrytz
Copy link
Member

lrytz commented Sep 20, 2015

Just saw @lihaoyi's discussion about memory usage over at com-lihaoyi/Ammonite#216. It might be worth taking a look at the impact on memory usage of this PR.

@retronym
Copy link
Member Author

Regarding memory, we create and discard the presentation compiler at each press of TAB. This is in addition to the resident compiler that is held for the duration of the REPL session. I think this is fairly low risk: for applications that are embedding the interpreter and that might be memory sensitive, no cost is incurred (they don't trigger autocompletion). For interactive users with JLine, the short lifetime should avoid the extra objects surviving to the old generation.

@retronym
Copy link
Member Author

Heap usage looks okay to me. I was manually testing, autocompleting collections and the like:

image

@lihaoyi
Copy link
Contributor

lihaoyi commented Sep 20, 2015

FWIW my memory usage conclusion wasn't wrt total memory held, but memory retention behavior. You guys set a max of 256mb somewhere in your she'll script, which works great, but I wanted to leave Ammonite's open ended because I've used it for some relatively large non-memory small-dara work.

The interesting conclusion is that G1 keeps the heap smallish when the working set is smallish, whereas the default GC let's it grow to the max heap size, e.g. 4gb on my laptop, even if the working set never goes above 100-200mb.

Ammonite keeps the pressy around but initializes it lazily, so it isn't part of that trial either =)

@lrytz
Copy link
Member

lrytz commented Sep 20, 2015

thanks for checking the heap usage. i went through all the commits, it looks excellent. like everybody else, i can't wait to have it in :)

@lrytz
Copy link
Member

lrytz commented Sep 20, 2015

LGTM

@lrytz
Copy link
Member

lrytz commented Sep 20, 2015

/synch - maybe that helps with the cla status? -- yep it did -- but it also made scala-jenkins remove the "reviewed" label -- and it's quite stubborn about it.

@retronym
Copy link
Member Author

@lrytz I've removed the unused parameter you flagged and made another minor refactoring in the last commit.

@lrytz
Copy link
Member

lrytz commented Sep 21, 2015

LGTM - Let's Get This Merged :)

lrytz added a commit that referenced this pull request Sep 21, 2015
@lrytz lrytz merged commit 5e86270 into scala:2.11.x Sep 21, 2015
@lrytz
Copy link
Member

lrytz commented Sep 21, 2015

🍰 !

@som-snytt
Copy link
Contributor

💦 what an effort!

🐫 crosses the REPL waste and also handles 🐫 case.

🕦 is when he was up checking the heap.

@som-snytt
Copy link
Contributor

The commit for SI-1931 went in under #4554

@@ -288,6 +290,8 @@ trait CompilerControl { self: Global =>
accessible: Boolean,
inherited: Boolean,
viaView: Symbol) extends Member {
// should be a case class parameter, but added as a var instead to preserve compatibility with the IDE
var prefix: Type = NoType
Copy link
Contributor

Choose a reason for hiding this comment

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

I think source-compatibility should be ok... Do we pattern match on this? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think you do. We'd need to phase that out first
On Thu, 24 Sep 2015 at 6:05 PM, Iulian Dragos notifications@github.com
wrote:

In src/interactive/scala/tools/nsc/interactive/CompilerControl.scala
#4725 (comment):

@@ -288,6 +290,8 @@ trait CompilerControl { self: Global =>
accessible: Boolean,
inherited: Boolean,
viaView: Symbol) extends Member {

  • // should be a case class parameter, but added as a var instead to preserve compatibility with the IDE
  • var prefix: Type = NoType

I think source-compatibility should be ok... Do we pattern match on this?
:)


Reply to this email directly or view it on GitHub
https://github.com/scala/scala/pull/4725/files#r40294033.

@retronym retronym changed the title Topic/completely 2.11 Use presentation compiler for REPL tab completion Oct 6, 2015
@retronym retronym added the release-notes worth highlighting in next release notes label Oct 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/scala/scala/pull/4725

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy