@nikomatsakis design doc drafted. :-)
written it in the first person, because I thought that made sense.
hopefully it has a lot of the sort of stuff you wanted anyway.
(you should be able to comment or edit BTW)
Perhaps submit this as a design meeting proposal for discussion?
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. :-)
@Alexander Regueiro I'll create a "to do" item to review this in more depth -- first person seems fine though!
hmm, skimmed briefly
whta I think I'd like to see a bit of is
what you think the "big picture" is
like, maybe walk through how the repl would work ?
I am imagining something like this:
cc @Alexander Regueiro :point_up:
so you want more about the mechanics of the actual REPL (even if that's outside rustc), eh? @nikomatsakis
yeah -- I mean I don't want to know too many details, but I want to get the "forest" and not just the "trees"
that would help to understand why the changes are needed, not just what they are
@nikomatsakis well the second section talks about the actual changes and why they're needed, to be fair.
but I can definitely appreciate wanting a higher-level picture too
so I'll add a section before that, and ping you again when it's done
@nikomatsakis (and others)... updated the doc with a general overview of how the REPL works. hope that helps.
feel free to edit it or leave notes now!
or just ask me questions, and we can update it together.
Maybe it's a stupid question, but how will external crates fit in the story?
@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.
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)
@Laurențiu Nicola Incr. comp. already uses the FS right now. I'm providing an alternative for REPL use (at least mainly).
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?
It's not there yet, but not super far away. It would get cached in RAM though at present, yes.
a per-user build cache would be useful yes. have you talked with others about it?
No, but it's a long-standing request, I think, e.g. https://github.com/rust-lang/cargo/issues/5931
it kind-of already exists for things that are cargo-installed
so that would "just" need to be extended
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.
but that's not a big concern right now
Oh course, it's just something to think about before shipping it
it's very much a WIP
so there's no "shipping" per se. only an alpha release.
which will hopefully come in the next month
just needs the rustc bits integrated now
I can't wait to try it out :)
is this thread the discussion for PR #64648?
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.
@DPC yep and future PRs too, which I can create when everything gets approved.
@Josh Triplett Quite possibly yes. I suspect we’ll get around discuss this after the initial release.
@nikomatsakis Let me know when you’ve had time to review things.
@Alexander Regueiro btw, I'm skimming the doc now in between making breakfast :) the new section helps a lot, thanks
@nikomatsakis Cool, glad to hear.
@nikomatsakis got one point I want to discuss about that design doc, but apart from that, would like to FCP it soon really
@nikomatsakis sorry to bother you again... got a few mins today?
There's an issue on the compiler-team repo now, for anyone following: https://github.com/rust-lang/compiler-team/issues/213
@Alexander Regueiro Hm, so I expected a few more of the bullets to be non-essential
in particular e.g. "Added an interface for injecting a pre-parsed “user body” into the parsing pipeline. "
since that's presumably nothing more than a performance optimization?
I can't imagine parsing being that slow...
No, it’s necessary.
So that the user body be injected at the right point.
But you don't need the preparsing logic
Like, you are already modifying the AST to insert the attribute
Why can't you... insert the body instead?
Or, I guess, to clarify, you'd still perhaps pre parse - but the compiler wouldn't need to know
there would still need to be some sort of small change to the parser though
to allow 'hooks'
Hm, I would expect you could grab the ast::Crate (I assume you have a custom driver?) and modify that
yes I used to do that
but it's ugly as hell. you have to effectively "hard code" the path in the AST to the user fn body
you can't use a
Visitor, because that's non-mutable
hm, we definitely have a way to modify the AST
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)
er, ignore the branch
yeah that has a weird API, that works around "mapping"
rather than in-place modification
not quite what we want
well, you don't really care, in theory?
maybe I should take a look at your repo or something
I feel like I'm surprised by decisions you've made and there's probably good reasons I'm just not seeing :)
@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).
so that's pretty surprising to me, to be honest, just because e.g. Serde does this "item insertion" and such all the time
@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).
anyway, would you be opposed to me trying to put up a PR or two or so?
ah, I guess you've not published it yet
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 :)
@simulacrum of course not :-)
@simulacrum not put up the REPL yet, no, though my rustc fork is up (see the PR you reviewed, initial comment)
yeah I suspect the REPL repo would be more helpful
@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.
(feel free to ping/remind me)
@Alexander Regueiro Can you share the repo with me in some way? I don't think I see anything in email
@simulacrum Okay, pushed to GH and invited you.
Thanks! Will take a look soon.
@simulacrum Note that it's missing the UI still (just has some dummy input "lines" or eval sessions, but it should be very straightforward)
adding the UI on top of this is a routine job, though will take a bit of time. :-) nothing to worry about regardlss.
@simulacrum Let me know if you have any questions BTW
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.
@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
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?
@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.
Anyway glad you understand it better.
hm, maybe I started fixing before I compiled or something, I'll try and take another look (rebasing would be good anyway, I presume)
Already rebased the rustc changes several times though, only going to do it again to open new PRs when we're ready :-P
and yes it's a fork of miri right now
can't quite be used as a library unfortunately
because I change the Miri "machine" usd.
also, I add a new intrinsic
but modulo those two things, I think it's basically the same base
eventually it would be nice to add support to miri for those extensions
should be relatively minor
but no need to do it right now
makes sense -- was hopeful that miri'd be flexible enough to allow that as a library, but makes sense :)
yep, it's not far away at least
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
@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.
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
It's a neater, cleaner approach, and I think the one we want medium-term at the very least. :-)
Without it, I think we'd be creating more work for ourselves (especially me) in the long-run.
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.
@simulacrum Any update? :-)
I have not spent any more time this week I'm afraid
have been pretty busy
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
@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?
Uh, okay. I'm not sure - probably makes sense to get buy-in from e.g. @eddyb or @nikomatsakis and only then FCP..
@simulacrum yes, from one of them, ideally Niko. we can't launch FCPs anyway, as just contributors...
But I appreciate your thoughts & informal reviewing!
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.
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?
I guess it doesn't matter, but it doesn't have the appropriate label, at minimum :)
let me fix that
@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. :-)
@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.
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.