-
Notifications
You must be signed in to change notification settings - Fork 72
REPL meeting minutes. #226
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
nikomatsakis
merged 2 commits into
rust-lang:master
from
pnkfelix:2019-11-29-repl-meeting
Dec 5, 2019
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
--- | ||
title: Read-Eval-Print-Loop (REPL) Extensions | ||
type: docs | ||
--- | ||
|
||
* [Zulip stream][] or read on the [Zulip archive][]. | ||
|
||
[Zulip stream]: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/design.20meeting.202019-11-29/near/182183477 | ||
[Zulip archive]: https://zulip-archive.rust-lang.org/131828tcompiler/72407designmeeting20191129.html | ||
|
||
# Agenda | ||
|
||
- Design doc: https://hackmd.io/GJokfI0wQ0i4SIgRbFTmfw | ||
|
||
- Big picture overview and questions | ||
|
||
- Essential changes to support REPL extensions | ||
|
||
# Big picture overview of design, and on-the-fly questions about it | ||
|
||
- Read (to an AST), Compile (AST to MIR), Eval (the MIR to a value), Print (the value). and Loop. | ||
- Question: Is this going to be a tool analogous to miri, where check-in's that break it may not break CI immediately, but rather will just file follow-up bugs? | ||
- Answer 1: it has to be a tool; it depends on miri (or a fork of miri) | ||
- Answer 2: it will be even less than a submodule (where a CI automatically files follow-up bugs): There | ||
won't be any automatic filing of follow-up bugs (alexreg has said | ||
they will handle maintenance manually); this is how miri and | ||
clippy had initially started. It could become a submodule later. | ||
|
||
- Question: Does this mean if we (rustc developers) make breaking | ||
changes to the compiler, we won't necessarily know, right? (Unless | ||
explicit tests are added to rustc test suite.) | ||
- Answer 1: Yes, but this is considered acceptable | ||
- Answer 2: We can annotate relevant points in `rustc` with `// REPL` comments, | ||
(e.g. to avoid them being made private or removed due to lack of use in rustc). | ||
|
||
- Design doc is split into (interleaved) essential and non-essential changes | ||
|
||
- For remainder of meeting, we reviewed each essential change at a high level, | ||
and asked participatnts about their thoughts on that change. | ||
|
||
# Add an “interpreter mode” to the compiler interface | ||
|
||
- E.g. conditionally prevents dead user variable elimination. | ||
- for example, when you do `let x = ...` in the repl, you want `x` to be kept around for the future repl inputs. | ||
- Question: Are there other contexts where one would the dead-user-variable-elim optimization disable? | ||
- Answer 1: Probably not | ||
- Answer 2: Patching a running binary, perhaps | ||
|
||
- Question: What deos it mean to prepend local variables from previous evaluation sessions? | ||
- Answer: For each live variable add `#[rustc_repl_alive] let var: type;` | ||
|
||
- Conclusion: No one objects to the addition of the "interpreter mode" flag | ||
|
||
# Slightly expanded miri API | ||
|
||
|
||
- "Added insert_alloc method to machine, used by REPL for restoring memory when deserialising previous evaluation session." | ||
- There was some debate about exactly where this would need to go, but | ||
everyone agreed that the functionality was necessary. | ||
|
||
- "Added hooks before_statement, after_statement, before_stack_push (renamed existing method), after_stack_push, before_stack_pop, after_stack_pop (renamed existing method)." | ||
- no objections to these additions | ||
|
||
- "Made stack pop behaviour more flexible, so as to allow the cleanup flag to be independent of wherever the action is null or a “goto”." | ||
- Question: What are the stack pop changes? | ||
- Answer: it's important that we can exit a block / pop off the stack but still not perform cleanup (so as to be able to serialise the values of the locals when execution is done). | ||
- Question: Is the cleanup the running of destructors? | ||
- Answer: more like clearing the local's memory | ||
- no objections to this addition | ||
|
||
# Any remainning conerns from stake holders? | ||
|
||
We reviewed some vague misgivings with certain design decisions, but | ||
no one had strong objections. | ||
|
||
Its not typical to accept changes like this for supporting other projects, | ||
but it is also not unheard of (e.g. miri, priroda, clippy). | ||
|
||
Conclusion: people are generally okay with moving forward with what | ||
alexreg laid out, at least the Essential portion |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*remaining