REPL, part 1: Added interpreter mode to compiler interface, interpreter parsing functionality #64648

alexreg · 2019-09-21T01:28:34Z

Summary:

Adds "interpreter mode" to compiler interface. This will be used later in several places, including diagnostics.
Adds "interpreter tag" to ast::Locals to track info like whether they came from a previous eval session or have been moved.
Added interface for injecting a pre-parsed "user body" into the parsing pipeline. cf. TyCtxt::get_interp_user_fn, expr.rs
Moved Steal data structure to rustc_data_structures crate since a) it was already used outside of rustc::ty crate, b) it's now used even a little more outside there.
Made a few more things pub (as little as possible), so the interpreter can use them.

If you want the big picture of where this is going in terms of compiler changes (probably 2/3 more PRs needed), take a look at my personal branch. I will also be publishing the REPL repo itself soon.

Also, sorry for the lack of commits; I basically just carved this out of an even bigger squashed commit after much, much hacking! (It might be a tad heavy on cosmetic stuff too, for the same reason. If it's okay, then great, otherwise I can try to revert certain areas that people really don't want.)

Maybe @Centril / @Zoxc for review? Not wholly sure.

CC @nikomatsakis @mw @eddyb

alexreg · 2019-09-21T01:36:54Z

r? @Centril (provisional)

src/librustc/ty/context.rs

src/librustc_interface/interface.rs

src/librustc_interface/queries.rs

src/libsyntax/feature_gate/builtin_attrs.rs

src/libsyntax/parse/parser.rs

src/libsyntax/parse/parser/expr.rs

Centril · 2019-09-21T11:27:02Z

I left some thoughts above but most of this is outside my wheelhouse.

r? @Zoxc

src/libsyntax/feature_gate/builtin_attrs.rs

src/test/ui/reify-intrinsic.stderr

bors · 2019-09-23T06:50:42Z

☔ The latest upstream changes (presumably #64695) made this pull request unmergeable. Please resolve the merge conflicts.

Mark-Simulacrum · 2019-09-26T16:20:16Z

I would personally prefer we split the cosmetic changes out of this PR since it's orthogonal and can land separately (and is likely to generate more review traffic... after all, we're painting a shed :)

src/librustc/ty/context.rs

src/librustc_data_structures/steal.rs

src/libsyntax/ast.rs

src/libsyntax/parse/parser/expr.rs

src/test/ui/reify-intrinsic.stderr

Mark-Simulacrum · 2019-09-26T16:38:26Z

I think we might want T-compiler or so to provisionally agree to this -- it feels like the kind of thing we might want an eRFC (at least) for, but I also see why you might want to avoid that process.

Modulo possibly wanting an RFC, and splitting out the cosmetic change commit, I think that I at least feel that this PR is good enough (with my review comments addressed).

Centril · 2019-09-26T16:46:16Z

An RFC feels rather overkill. We didn't have ones for Clippy or Miri when they wanted to hook into the compiler and I don't see why we'd need one now.

alexreg · 2019-09-26T16:55:16Z

@Mark-Simulacrum Thanks for the review!

I very much agree with @Centril about not needing an RFC for this (I'm naturally disinclined to them for minor changes and experimentation, and I think @nikomatsakis feels the same. Also with @Centril a.k.a. the unofficial King of RFCs is typically more inclined to them, so if even he doesn't want one...).

Let me address your comments as much as possible over the next day (I left some replies too), which mainly make sense to me, and then you can have another look if that's alright. As for cosmetic stuff, some of them are very much "intertwined" with the functionality code, and I feel it would be a pain to factor them out... also, there's a policy specifically against accepting separate cosmetic PRs these days. So I'd implore you to let these stay in, if possible, although if you/other team members want me to trim them down, let me know which ones, and I can probably do that without much of a problem. :-)

Mark-Simulacrum · 2019-09-26T17:27:23Z

Left some replies to your comments. If you ping me on Zulip I'll almost certainly respond quickly most of the time, Discord is a bit more flaky with notifications; but either works if you want to talk about anything I've said synchronously.

Yeah, the eRFC/RFC comment was mostly for the larger feature, I agree we don't need it for the initial experimentation, if it has little impact on compiler and such.

@Centril

…crum REPL, part 1: Added interpreter mode to compiler interface, interpreter parsing functionality Summary: * Adds "interpreter mode" to compiler interface. This will be used later in several places, including diagnostics. * Adds "interpreter tag" to `ast::Local`s to track info like whether they came from a previous eval session or have been moved. * Added interface for injecting a pre-parsed "user body" into the parsing pipeline. cf. `TyCtxt::get_interp_user_fn`, `expr.rs` * Moved `Steal` data structure to `rustc_data_structures` crate since a) it was already used outside of `rustc::ty` crate, b) it's now used even a little more outside there. * Made a few more things `pub` (as little as possible), so the interpreter can use them. If you want the big picture of where this is going in terms of compiler changes (probably 2/3 more PRs needed), take a look at my [personal branch](https://github.com/alexreg/rust/tree/rush). I will also be publishing the REPL repo itself soon. Also, sorry for the lack of commits; I basically just carved this out of an even bigger squashed commit after much, much hacking! (It might be a tad heavy on cosmetic stuff too, for the same reason. If it's okay, then great, otherwise I can try to revert certain areas that people really don't want.) Maybe @Centril / @Zoxc for review? Not wholly sure. CC @nikomatsakis @mw @eddyb

@Centril

…crum REPL, part 1: Added interpreter mode to compiler interface, interpreter parsing functionality Summary: * Adds "interpreter mode" to compiler interface. This will be used later in several places, including diagnostics. * Adds "interpreter tag" to `ast::Local`s to track info like whether they came from a previous eval session or have been moved. * Added interface for injecting a pre-parsed "user body" into the parsing pipeline. cf. `TyCtxt::get_interp_user_fn`, `expr.rs` * Moved `Steal` data structure to `rustc_data_structures` crate since a) it was already used outside of `rustc::ty` crate, b) it's now used even a little more outside there. * Made a few more things `pub` (as little as possible), so the interpreter can use them. If you want the big picture of where this is going in terms of compiler changes (probably 2/3 more PRs needed), take a look at my [personal branch](https://github.com/alexreg/rust/tree/rush). I will also be publishing the REPL repo itself soon. Also, sorry for the lack of commits; I basically just carved this out of an even bigger squashed commit after much, much hacking! (It might be a tad heavy on cosmetic stuff too, for the same reason. If it's okay, then great, otherwise I can try to revert certain areas that people really don't want.) Maybe @Centril / @Zoxc for review? Not wholly sure. CC @nikomatsakis @mw @eddyb

Centril · 2019-10-01T12:21:28Z

Failed in #64954 (comment), @bors r-

oli-obk · 2019-10-01T13:01:34Z

Before merging this we'll do a design meeting of the compiler team to figure out how and whether the compiler should be modified in order to help creating a REPL. Relevant discussion happened on zulip.

Mark-Simulacrum · 2019-10-01T13:55:06Z

r? @oli-obk on this PR as the compiler team driver for now then

alexreg · 2019-10-01T16:57:06Z

Yeah, just noticed this now. Thanks for your reviews anyway @Mark-Simulacrum! Hopefully we can get it merged soon, regardless (at least in a form close to the present one).

bors · 2019-10-02T10:06:17Z

☔ The latest upstream changes (presumably #64981) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2019-10-09T18:45:59Z

Hey all -- sorry for being silent. @alexreg pointed me at this a while back but I've been behind. In general, I agree that a design meeting would be great and appropriate. Earlier, @alexreg wrote:

If you want the big picture of where this is going in terms of compiler changes (probably 2/3 more PRs needed), take a look at my personal branch. I will also be publishing the REPL repo itself soon.

I think that having an idea of the bigger target is exactly the kind of thing we should talk about, but unfortunately I don't really have the time to read a branch and try to "reverse engineer" what the larger design is from that. What I would like to see is probably a more elaborated version of the "summary" from this PR, that tries to show how the pieces will fit together in the end.

I'm not sure yet overall whether I want to land support for a REPL. I think a REPL would be a great thing. But we have a lot of stuff going on and a lot of priorities. So I guess it's partly a question of how "invasive" the support is, and what kind of ongoing support it seems likely to require. My impression from this PR is that it should be relatively minimal? This seems like the kind of information that we would be looking for in evaluating the design doc.

(I'd also like the design doc to be something that could move to rustc-guide to help people in understanding the changes that are being done. I personally find it really hard to review commits when I don't have a reasonable up-front idea of what the PR is trying to do.)

nikomatsakis · 2019-10-09T18:46:26Z

@alexreg to be clear, the design meeting process is pretty lightweight! Just file an issue on the compiler-team repo. We can also schedule it at a time that's not the normal slot, if that slot doesn't work well for you.

alexreg · 2019-10-10T01:03:06Z

@nikomatsakis

I think that having an idea of the bigger target is exactly the kind of thing we should talk about, but unfortunately I don't really have the time to read a branch and try to "reverse engineer" what the larger design is from that. What I would like to see is probably a more elaborated version of the "summary" from this PR, that tries to show how the pieces will fit together in the end.

That's fair. And I didn't expect you to do as much. I just wrote this to say "hey, the branch is there in case you're curious". What I can do is write up an expanded summary doc for this PR and the upcoming ones, and the overall vision. On HackMD or something.

I'm not sure yet overall whether I want to land support for a REPL. I think a REPL would be a great thing. But we have a lot of stuff going on and a lot of priorities. So I guess it's partly a question of how "invasive" the support is, and what kind of ongoing support it seems likely to require. My impression from this PR is that it should be relatively minimal? This seems like the kind of information that we would be looking for in evaluating the design doc.

I can understand why, but I think if the intrusion into the rustc compiler is minimal (as I've planned it to be), then why not? I'm happy to take on "maintenance duties" as far as the few REPL-only bits of rustc are concerned. I'm convinced this PR is pretty unobtrusive, so I'm glad you agree there so far.

(I'd also like the design doc to be something that could move to rustc-guide to help people in understanding the changes that are being done. I personally find it really hard to review commits when I don't have a reasonable up-front idea of what the PR is trying to do.)

Yeah, I'll keep that in mind when writing it. If it's on HackMD, then others like you can add notes and adjust things here and there if you fancy... no obligation though.

@alexreg to be clear, the design meeting process is pretty lightweight! Just file an issue on the compiler-team repo. We can also schedule it at a time that's not the normal slot, if that slot doesn't work well for you.

Ah, didn't know, but sounds good. If we could somehow schedule a short one next week (giving me time to prepare this doc, and people to see it, but not so longer as to let this PR linger more or even fester), then perfect.

mark-i-m · 2019-10-10T03:28:39Z

Ah, didn't know, but sounds good. If we could somehow schedule a short one next week (giving me time to prepare this doc, and people to see it, but not so longer as to let this PR linger more or even fester), then perfect.

@alexreg I believe the design meetings are planned out a month in advance (e.g. the next three weekly meeting topics have already been chosen). I might be wrong though... I've mostly been lurking...

Btw, this is really cool, and I would love to see a repl happen...

nikomatsakis · 2019-10-11T17:08:44Z

We pick the "whole compiler" design meetings every 4 weeks, yes, but we're part-way through the cycle. @alexreg if you file the issue now we'd be picking the next set on October 25th. You can read more about the process on the compiler-team web page.

alexreg · 2019-10-16T15:53:45Z

src/libsyntax/parse/parser/expr.rs

+    /// In interpreter mode, parses the placeholder for the user fn body as the user-provided
+    /// block (not from the source).
+    /// Outside of interpreter mode, simply returns `Ok(None)`.
+    fn parse_interp_user_fn_block(&mut self) -> PResult<'a, Option<P<Block>>> {


Reminder: check for interp mode here, maybe?

Dylan-DPC-zz · 2020-03-06T18:52:30Z

~~I'm closing this due to inactivity and the large number of conflicts. Feel free to reopen it (or preferably create a new PR ) when you have progress :) Thanks~~ Nevermind, didn't realise it was blocked

bjorn3 · 2020-03-06T19:12:37Z

The design meetong referenced in #64648 (comment) has already happened a while ago.

bors · 2020-05-01T07:55:26Z

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout rush-parsing (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self rush-parsing --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

CONFLICT (modify/delete): src/libsyntax_pos/symbol.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax_pos/symbol.rs left in tree.
CONFLICT (modify/delete): src/libsyntax_pos/span_encoding.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax_pos/span_encoding.rs left in tree.
CONFLICT (modify/delete): src/libsyntax_ext/deriving/debug.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax_ext/deriving/debug.rs left in tree.
CONFLICT (modify/delete): src/libsyntax/parse/parser/stmt.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax/parse/parser/stmt.rs left in tree.
CONFLICT (modify/delete): src/libsyntax/parse/parser/expr.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax/parse/parser/expr.rs left in tree.
CONFLICT (modify/delete): src/libsyntax/parse/parser.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax/parse/parser.rs left in tree.
CONFLICT (modify/delete): src/libsyntax/parse/mod.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax/parse/mod.rs left in tree.
CONFLICT (modify/delete): src/libsyntax/parse/diagnostics.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax/parse/diagnostics.rs left in tree.
CONFLICT (modify/delete): src/libsyntax/mut_visit.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax/mut_visit.rs left in tree.
CONFLICT (modify/delete): src/libsyntax/feature_gate/builtin_attrs.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax/feature_gate/builtin_attrs.rs left in tree.
CONFLICT (modify/delete): src/libsyntax/ext/build.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax/ext/build.rs left in tree.
CONFLICT (modify/delete): src/libsyntax/ast.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/libsyntax/ast.rs left in tree.
Auto-merging src/librustc_mir/transform/mod.rs
CONFLICT (content): Merge conflict in src/librustc_mir/transform/mod.rs
Auto-merging src/librustc_interface/queries.rs
CONFLICT (content): Merge conflict in src/librustc_interface/queries.rs
Auto-merging src/librustc_interface/passes.rs
CONFLICT (content): Merge conflict in src/librustc_interface/passes.rs
Auto-merging src/librustc_interface/lib.rs
CONFLICT (content): Merge conflict in src/librustc_interface/lib.rs
Auto-merging src/librustc_interface/interface.rs
CONFLICT (content): Merge conflict in src/librustc_interface/interface.rs
CONFLICT (rename/delete): src/librustc/ty/steal.rs deleted in HEAD and renamed to src/librustc_data_structures/steal.rs in heads/homu-tmp. Version heads/homu-tmp of src/librustc_data_structures/steal.rs left in tree.
Auto-merging src/librustc_data_structures/lib.rs
CONFLICT (content): Merge conflict in src/librustc_data_structures/lib.rs
CONFLICT (modify/delete): src/librustc/ty/query/mod.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/librustc/ty/query/mod.rs left in tree.
CONFLICT (modify/delete): src/librustc/ty/mod.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/librustc/ty/mod.rs left in tree.
CONFLICT (modify/delete): src/librustc/ty/context.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/librustc/ty/context.rs left in tree.
CONFLICT (modify/delete): src/librustc/session/config.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/librustc/session/config.rs left in tree.
CONFLICT (modify/delete): src/librustc/query/mod.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/librustc/query/mod.rs left in tree.
CONFLICT (modify/delete): src/librustc/ich/impls_ty.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/librustc/ich/impls_ty.rs left in tree.
CONFLICT (modify/delete): src/librustc/ich/impls_hir.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/librustc/ich/impls_hir.rs left in tree.
CONFLICT (modify/delete): src/librustc/hir/mod.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/librustc/hir/mod.rs left in tree.
CONFLICT (modify/delete): src/librustc/hir/lowering.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/librustc/hir/lowering.rs left in tree.
CONFLICT (modify/delete): src/librustc/arena.rs deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of src/librustc/arena.rs left in tree.
warning: inexact rename detection was skipped due to too many files.
warning: you may want to set your merge.renamelimit variable to at least 3410 and retry the command.
Automatic merge failed; fix conflicts and then commit the result.

nikomatsakis · 2020-05-21T21:50:20Z

I'm going to close this for inactivity.

rust-highfive assigned zackmdavis Sep 21, 2019