Stream: general

Topic: REPL


Alexander Regueiro (Oct 16 2019 at 16:28, on Zulip):

@nikomatsakis design doc drafted. :-)

Alexander Regueiro (Oct 16 2019 at 16:28, on Zulip):

https://hackmd.io/@alexreg/rustc-repl-modifications-design-doc

Alexander Regueiro (Oct 16 2019 at 16:28, on Zulip):

written it in the first person, because I thought that made sense.

Alexander Regueiro (Oct 16 2019 at 16:29, on Zulip):

hopefully it has a lot of the sort of stuff you wanted anyway.

Alexander Regueiro (Oct 16 2019 at 16:32, on Zulip):

(you should be able to comment or edit BTW)

davidtwco (Oct 16 2019 at 16:39, on Zulip):

Perhaps submit this as a design meeting proposal for discussion?

Alexander Regueiro (Oct 16 2019 at 19:04, on Zulip):

Fair point, which we considered, though @nikomatsakis and @simulacrum and I chatted last week, and the current idea is to basically FCP this and only do a design meeting if there are substantial issues. :-)

nikomatsakis (Oct 16 2019 at 19:08, on Zulip):

@Alexander Regueiro I'll create a "to do" item to review this in more depth -- first person seems fine though!

nikomatsakis (Oct 16 2019 at 19:08, on Zulip):

hmm, skimmed briefly

nikomatsakis (Oct 16 2019 at 19:08, on Zulip):

whta I think I'd like to see a bit of is

nikomatsakis (Oct 16 2019 at 19:08, on Zulip):

what you think the "big picture" is

nikomatsakis (Oct 16 2019 at 19:08, on Zulip):

like, maybe walk through how the repl would work ?

nikomatsakis (Oct 16 2019 at 19:09, on Zulip):

I am imagining something like this:

nikomatsakis (Oct 16 2019 at 19:09, on Zulip):
nikomatsakis (Oct 16 2019 at 19:09, on Zulip):
nikomatsakis (Oct 16 2019 at 19:10, on Zulip):
nikomatsakis (Oct 16 2019 at 19:10, on Zulip):

cc @Alexander Regueiro :point_up:

Alexander Regueiro (Oct 16 2019 at 19:10, on Zulip):

ah right

Alexander Regueiro (Oct 16 2019 at 19:11, on Zulip):

so you want more about the mechanics of the actual REPL (even if that's outside rustc), eh? @nikomatsakis

nikomatsakis (Oct 16 2019 at 19:14, on Zulip):

yeah -- I mean I don't want to know too many details, but I want to get the "forest" and not just the "trees"

nikomatsakis (Oct 16 2019 at 19:15, on Zulip):

that would help to understand why the changes are needed, not just what they are

Alexander Regueiro (Oct 16 2019 at 19:21, on Zulip):

@nikomatsakis well the second section talks about the actual changes and why they're needed, to be fair.

Alexander Regueiro (Oct 16 2019 at 19:21, on Zulip):

but I can definitely appreciate wanting a higher-level picture too

Alexander Regueiro (Oct 16 2019 at 19:21, on Zulip):

so I'll add a section before that, and ping you again when it's done

Alexander Regueiro (Oct 16 2019 at 23:28, on Zulip):

@nikomatsakis (and others)... updated the doc with a general overview of how the REPL works. hope that helps.

Alexander Regueiro (Oct 16 2019 at 23:29, on Zulip):

feel free to edit it or leave notes now!

Alexander Regueiro (Oct 16 2019 at 23:29, on Zulip):

or just ask me questions, and we can update it together.

Laurențiu Nicola (Oct 17 2019 at 07:16, on Zulip):

Maybe it's a stupid question, but how will external crates fit in the story?

Alexander Regueiro (Oct 17 2019 at 17:11, on Zulip):

@Laurențiu Nicola Not stupid at all. Basically because it’s built on miri, the capabilities will be there to use external crates without much extra work... but I haven’t explicitly added support for it yet.

Laurențiu Nicola (Oct 17 2019 at 17:12, on Zulip):

But you mention a fs cache for incremental compilation. Would that be used for external crates too? I wish we had a per-user build cache (: (in cargo, not only for the REPL)

Alexander Regueiro (Oct 17 2019 at 17:50, on Zulip):

@Laurențiu Nicola Incr. comp. already uses the FS right now. I'm providing an alternative for REPL use (at least mainly).

Laurențiu Nicola (Oct 17 2019 at 17:52, on Zulip):

Got it, but is it a good idea? What if I extern crate serde; in the REPL, will all of it get cached to RAM? Or is extern crate support in the REPL very far away?

Alexander Regueiro (Oct 17 2019 at 17:53, on Zulip):

It's not there yet, but not super far away. It would get cached in RAM though at present, yes.

Alexander Regueiro (Oct 17 2019 at 17:53, on Zulip):

a per-user build cache would be useful yes. have you talked with others about it?

Laurențiu Nicola (Oct 17 2019 at 17:54, on Zulip):

No, but it's a long-standing request, I think, e.g. https://github.com/rust-lang/cargo/issues/5931

Alexander Regueiro (Oct 17 2019 at 17:55, on Zulip):

it kind-of already exists for things that are cargo-installed

Alexander Regueiro (Oct 17 2019 at 17:55, on Zulip):

so that would "just" need to be extended

Alexander Regueiro (Oct 17 2019 at 17:57, on Zulip):

also, ideally when it comes to incr. comp., we could allow for separate "backends" (on-disk vs. in-memory) for the local crate and extern crates.

Alexander Regueiro (Oct 17 2019 at 17:57, on Zulip):

but that's not a big concern right now

Laurențiu Nicola (Oct 17 2019 at 17:58, on Zulip):

Oh course, it's just something to think about before shipping it

Alexander Regueiro (Oct 17 2019 at 18:12, on Zulip):

it's very much a WIP

Alexander Regueiro (Oct 17 2019 at 18:12, on Zulip):

so there's no "shipping" per se. only an alpha release.

Alexander Regueiro (Oct 17 2019 at 18:13, on Zulip):

which will hopefully come in the next month

Alexander Regueiro (Oct 17 2019 at 18:13, on Zulip):

just needs the rustc bits integrated now

gnzlbg (Oct 18 2019 at 08:21, on Zulip):

I can't wait to try it out :)

DPC (Oct 18 2019 at 09:03, on Zulip):

is this thread the discussion for PR #64648?

Josh Triplett (Oct 18 2019 at 09:38, on Zulip):

Side note regarding REPLs: based on some discussions at a past RustConf, I think there are ways to improve type inference specifically for a REPL, to handle cases that would otherwise require early type signatures.

Alexander Regueiro (Oct 18 2019 at 15:01, on Zulip):

@DPC yep and future PRs too, which I can create when everything gets approved.

Alexander Regueiro (Oct 18 2019 at 15:02, on Zulip):

@Josh Triplett Quite possibly yes. I suspect we’ll get around discuss this after the initial release.

Alexander Regueiro (Oct 21 2019 at 16:04, on Zulip):

@nikomatsakis Let me know when you’ve had time to review things.

nikomatsakis (Oct 24 2019 at 10:26, on Zulip):

@Alexander Regueiro btw, I'm skimming the doc now in between making breakfast :) the new section helps a lot, thanks

Alexander Regueiro (Oct 24 2019 at 14:22, on Zulip):

@nikomatsakis Cool, glad to hear.

Alexander Regueiro (Oct 25 2019 at 15:12, on Zulip):

@nikomatsakis got one point I want to discuss about that design doc, but apart from that, would like to FCP it soon really

Alexander Regueiro (Oct 28 2019 at 17:25, on Zulip):

@nikomatsakis sorry to bother you again... got a few mins today?

Alexander Regueiro (Nov 02 2019 at 11:42, on Zulip):

There's an issue on the compiler-team repo now, for anyone following: https://github.com/rust-lang/compiler-team/issues/213

simulacrum (Nov 07 2019 at 18:49, on Zulip):

@Alexander Regueiro Hm, so I expected a few more of the bullets to be non-essential

simulacrum (Nov 07 2019 at 18:49, on Zulip):

in particular e.g. "Added an interface for injecting a pre-parsed “user body” into the parsing pipeline. "

simulacrum (Nov 07 2019 at 18:50, on Zulip):

since that's presumably nothing more than a performance optimization?

simulacrum (Nov 07 2019 at 18:50, on Zulip):

I can't imagine parsing being that slow...

Alexander Regueiro (Nov 07 2019 at 20:07, on Zulip):

No, it’s necessary.

Alexander Regueiro (Nov 07 2019 at 20:07, on Zulip):

So that the user body be injected at the right point.

Alexander Regueiro (Nov 07 2019 at 20:54, on Zulip):

@simulacrum

simulacrum (Nov 07 2019 at 20:59, on Zulip):

Hm

simulacrum (Nov 07 2019 at 21:00, on Zulip):

But you don't need the preparsing logic

simulacrum (Nov 07 2019 at 21:00, on Zulip):

Right?

simulacrum (Nov 07 2019 at 21:00, on Zulip):

Like, you are already modifying the AST to insert the attribute

simulacrum (Nov 07 2019 at 21:01, on Zulip):

Why can't you... insert the body instead?

simulacrum (Nov 07 2019 at 21:09, on Zulip):

Or, I guess, to clarify, you'd still perhaps pre parse - but the compiler wouldn't need to know

Alexander Regueiro (Nov 07 2019 at 22:05, on Zulip):

Huh?

Alexander Regueiro (Nov 07 2019 at 22:05, on Zulip):

Well...

Alexander Regueiro (Nov 07 2019 at 22:05, on Zulip):

well, possibly

Alexander Regueiro (Nov 07 2019 at 22:06, on Zulip):

there would still need to be some sort of small change to the parser though

Alexander Regueiro (Nov 07 2019 at 22:06, on Zulip):

to allow 'hooks'

simulacrum (Nov 07 2019 at 22:06, on Zulip):

Hm, I would expect you could grab the ast::Crate (I assume you have a custom driver?) and modify that

Alexander Regueiro (Nov 07 2019 at 22:06, on Zulip):

yes I used to do that

Alexander Regueiro (Nov 07 2019 at 22:07, on Zulip):

but it's ugly as hell. you have to effectively "hard code" the path in the AST to the user fn body

Alexander Regueiro (Nov 07 2019 at 22:07, on Zulip):

you can't use a Visitor, because that's non-mutable

simulacrum (Nov 07 2019 at 22:07, on Zulip):

hm, we definitely have a way to modify the AST

simulacrum (Nov 07 2019 at 22:08, on Zulip):

https://github.com/Mark-Simulacrum/rust/blob/stage0-step/src/libsyntax/mut_visit.rs

Alexander Regueiro (Nov 07 2019 at 22:08, on Zulip):

yes but it's either a) in a local fashion, b) via MutVisitor which can't be used here (I tried and it just doesn't fit the bill)

simulacrum (Nov 07 2019 at 22:08, on Zulip):

er, ignore the branch

Alexander Regueiro (Nov 07 2019 at 22:08, on Zulip):

yeah that has a weird API, that works around "mapping"

Alexander Regueiro (Nov 07 2019 at 22:08, on Zulip):

rather than in-place modification

Alexander Regueiro (Nov 07 2019 at 22:08, on Zulip):

not quite what we want

simulacrum (Nov 07 2019 at 22:08, on Zulip):

well, you don't really care, in theory?

simulacrum (Nov 07 2019 at 22:08, on Zulip):

maybe I should take a look at your repo or something

simulacrum (Nov 07 2019 at 22:09, on Zulip):

I feel like I'm surprised by decisions you've made and there's probably good reasons I'm just not seeing :)

Alexander Regueiro (Nov 07 2019 at 22:10, on Zulip):

@simulacrum basically I played around with a lot of these things, and the obvious choices were just not feasible (or had a major performance hit).

simulacrum (Nov 07 2019 at 22:11, on Zulip):

so that's pretty surprising to me, to be honest, just because e.g. Serde does this "item insertion" and such all the time

Alexander Regueiro (Nov 07 2019 at 22:11, on Zulip):

@simulacrum the best I could envisage here is pre-parsing the whole thing, but adding a macro_hook fn field to the Parser struct or such, so that when pre-parsing is done, this hook is used (but rustc itself doesn't use any hooks).

simulacrum (Nov 07 2019 at 22:11, on Zulip):

anyway, would you be opposed to me trying to put up a PR or two or so?

simulacrum (Nov 07 2019 at 22:12, on Zulip):

ah, I guess you've not published it yet

simulacrum (Nov 07 2019 at 22:12, on Zulip):

I'd be happy to spend some time trying to realize my thoughts are not going to work, plus I'd love to play around with it a bit :)

Alexander Regueiro (Nov 07 2019 at 22:51, on Zulip):

@simulacrum of course not :-)

Alexander Regueiro (Nov 07 2019 at 22:52, on Zulip):

@simulacrum not put up the REPL yet, no, though my rustc fork is up (see the PR you reviewed, initial comment)

simulacrum (Nov 07 2019 at 22:52, on Zulip):

yeah I suspect the REPL repo would be more helpful

Alexander Regueiro (Nov 07 2019 at 23:02, on Zulip):

@simulacrum I haven't compiled it against my rustc build for a while now... it should still work in theory. have a start whenever you like, and I'll see what I can sort out with that soon.

Alexander Regueiro (Nov 07 2019 at 23:02, on Zulip):

(feel free to ping/remind me)

simulacrum (Nov 07 2019 at 23:17, on Zulip):

@Alexander Regueiro Can you share the repo with me in some way? I don't think I see anything in email

Alexander Regueiro (Nov 07 2019 at 23:20, on Zulip):

@simulacrum Okay, pushed to GH and invited you.

simulacrum (Nov 07 2019 at 23:20, on Zulip):

Thanks! Will take a look soon.

Alexander Regueiro (Nov 07 2019 at 23:21, on Zulip):

@simulacrum Note that it's missing the UI still (just has some dummy input "lines" or eval sessions, but it should be very straightforward)

Alexander Regueiro (Nov 07 2019 at 23:21, on Zulip):

adding the UI on top of this is a routine job, though will take a bit of time. :-) nothing to worry about regardlss.

Alexander Regueiro (Nov 07 2019 at 23:21, on Zulip):

sure

Alexander Regueiro (Nov 08 2019 at 15:54, on Zulip):

@simulacrum Let me know if you have any questions BTW

simulacrum (Nov 10 2019 at 16:10, on Zulip):

Okay -- I spent some time trying to figure out what all is going on, though wasn't able to get super far as compiling either of the two isn't quite easy (i.e., the rustc branch seems to not be quite in sync with the repl repo) -- and neither seems to be up to date with "latest rustc."

From what I can tell without compiling, though, I think I'm better understanding the primitives that you want to add, though I'm not yet quite clear on how they all fit together....

It does feel like before we move forward on review into landing PRs into the compiler it'd be good to make sure what we land is actually good enough to support the REPL. But I personally feel somewhat more on board to make this happen then I did before looking at the components.

simulacrum (Nov 10 2019 at 19:20, on Zulip):

@Alexander Regueiro one other thought I had -- it'd be great to put up all the modifications you want to land (basically what's necessary to compile the repl) and bors try it such that at least people developing on linux can avoid the rustc rebuild to try and hack on the REPL

simulacrum (Nov 10 2019 at 19:21, on Zulip):

It also looks like the REPL repository is a "fork" of miri right now -- is that actually necessary? Or could we use miri as a library successfully as well?

Alexander Regueiro (Nov 10 2019 at 22:54, on Zulip):

@simulacrum it should be in sync I believe with the REPL repo, unless I made some very minor edits... I forget. It's trivial to remedy in any case I think.

Alexander Regueiro (Nov 10 2019 at 22:55, on Zulip):

Anyway glad you understand it better.

simulacrum (Nov 10 2019 at 22:55, on Zulip):

hm, maybe I started fixing before I compiled or something, I'll try and take another look (rebasing would be good anyway, I presume)

Alexander Regueiro (Nov 10 2019 at 22:55, on Zulip):

Already rebased the rustc changes several times though, only going to do it again to open new PRs when we're ready :-P

Alexander Regueiro (Nov 10 2019 at 22:55, on Zulip):

and yes it's a fork of miri right now

Alexander Regueiro (Nov 10 2019 at 22:55, on Zulip):

can't quite be used as a library unfortunately

Alexander Regueiro (Nov 10 2019 at 22:56, on Zulip):

because I change the Miri "machine" usd.

Alexander Regueiro (Nov 10 2019 at 22:56, on Zulip):

also, I add a new intrinsic

Alexander Regueiro (Nov 10 2019 at 22:56, on Zulip):

but modulo those two things, I think it's basically the same base

Alexander Regueiro (Nov 10 2019 at 22:56, on Zulip):

eventually it would be nice to add support to miri for those extensions

Alexander Regueiro (Nov 10 2019 at 22:56, on Zulip):

should be relatively minor

Alexander Regueiro (Nov 10 2019 at 22:56, on Zulip):

but no need to do it right now

simulacrum (Nov 10 2019 at 22:56, on Zulip):

makes sense -- was hopeful that miri'd be flexible enough to allow that as a library, but makes sense :)

Alexander Regueiro (Nov 10 2019 at 22:59, on Zulip):

yep, it's not far away at least

Alexander Regueiro (Nov 10 2019 at 22:59, on Zulip):

and I think Ralf & Oli would be okay with these minor changes to make it usable as a lib once they see a RPEL released

Alexander Regueiro (Nov 10 2019 at 22:59, on Zulip):

REPL, even

Alexander Regueiro (Nov 10 2019 at 23:04, on Zulip):

@simulacrum anyway I hope you can appreciate the design and implementation 80% or 90% without building (as it seems you can), but if you want to try getting it built, go for it... shouldn't be hard at all like I said, but if you're unsure about anything, ask me here.

simulacrum (Nov 10 2019 at 23:05, on Zulip):

sure, thanks!

simulacrum (Nov 10 2019 at 23:06, on Zulip):

it does still feel like it goes a bit too far (e.g., I find the integration into incremental unexpected, it seems like a more ad-hoc system would be fine) but it doesn't seem like a wrong approach either

Alexander Regueiro (Nov 10 2019 at 23:16, on Zulip):

It's a neater, cleaner approach, and I think the one we want medium-term at the very least. :-)

Alexander Regueiro (Nov 10 2019 at 23:16, on Zulip):

Without it, I think we'd be creating more work for ourselves (especially me) in the long-run.

Alexander Regueiro (Nov 10 2019 at 23:19, on Zulip):

Miri has to serialise all its memory anyway, which uses something very similar to incr. comp. (or crate metadata to a lesser extent). The same mechanisms.

Alexander Regueiro (Nov 12 2019 at 00:34, on Zulip):

s/Miri[/the REPL

Alexander Regueiro (Nov 13 2019 at 13:19, on Zulip):

@simulacrum Any update? :-)

simulacrum (Nov 13 2019 at 13:19, on Zulip):

I have not spent any more time this week I'm afraid

simulacrum (Nov 13 2019 at 13:19, on Zulip):

have been pretty busy

simulacrum (Nov 13 2019 at 13:22, on Zulip):

To be honest I'm not sure that I have much work left either, so not sure that there's anything necessarily waiting on me

Alexander Regueiro (Nov 13 2019 at 14:51, on Zulip):

@simulacrum no prob. And yes, sounds fair. I can update the bit in the design dic about the parsing infrastructure... but then hopefully we can just FCP?

simulacrum (Nov 13 2019 at 14:56, on Zulip):

Uh, okay. I'm not sure - probably makes sense to get buy-in from e.g. @eddyb or @nikomatsakis and only then FCP..

Alexander Regueiro (Nov 13 2019 at 21:13, on Zulip):

@simulacrum yes, from one of them, ideally Niko. we can't launch FCPs anyway, as just contributors...

Alexander Regueiro (Nov 13 2019 at 21:14, on Zulip):

But I appreciate your thoughts & informal reviewing!

nikomatsakis (Nov 15 2019 at 13:57, on Zulip):

Hey @Alexander Regueiro -- haven't forgotten about this, but I have only limited bandwidth available for technical review on the compiler. This is definitely part of the reason that the design meetings exist, to try and parcel that bandwidth out in a structured way. Right now I'm trying to prioritize looking at the trait upcasting branch, for example, and other traits work. But I'll take another look at your revised proposal and try to share some thoughts.

nikomatsakis (Nov 15 2019 at 13:58, on Zulip):

PS, if https://github.com/rust-lang/compiler-team/issues/213 is meant to be a meeting proposal, is there a reason you didn't use the template?

nikomatsakis (Nov 15 2019 at 13:58, on Zulip):

I guess it doesn't matter, but it doesn't have the appropriate label, at minimum :)

nikomatsakis (Nov 15 2019 at 13:58, on Zulip):

let me fix that

Alexander Regueiro (Nov 15 2019 at 16:26, on Zulip):

@nikomatsakis Ah no, it wasn't. I thought the idea was just to do a design doc and FCP that, and only have a meeting if there are major objections? So as not to delay it (even)more. :-)

Alexander Regueiro (Nov 15 2019 at 16:27, on Zulip):

@nikomatsakis Anyway fair enough. I appreciate the time you've given me on trait upcasting, and yes, let's prioritise this! If you can give me any thoughts, that would be great, thanks.

Alexander Regueiro (Nov 15 2019 at 16:28, on Zulip):

And while my personal time bandwidth is greater, I need to be honest that my mental exhaustion from having this drag on so long (let me emphasise, I'm definitely not blaming you or anyone else for this!) means this is getting a bit difficult for me now. But I'll see what I can do.

Last update: Nov 21 2019 at 23:25UTC