-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Support call and drop terminators in custom mir #105814
Conversation
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
758ca80
to
3d849ae
Compare
//! let popped; | ||
//! | ||
//! { | ||
//! Call(unused, pop, Vec::push(v, value)) |
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.
I think it would be neat if we actually supported
//! unused = Vec::push(v, value) -> pop
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.
Yeah, I thought about this. There's a couple reasons I'm hesitant to do it:
- We'd need to use
=>
instead of->
, because of follow set rules. That's fine, although it's not the most intuitive syntax - More macros means worse error messages
- Supporting cleanup blocks. Presumably we will eventually want to support those, and I'm not sure what that would look like in connection with this, especially once Refactor unwind in MIR #102906 lands. Possibly
-> pop, Unreachable()
,-> pop, Abort()
, or-> pop, other_block
are fine though?
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.
Oh yea, I don't think that syntax should happen in this PR and at best any such syntax should desugar to what you have now.
|
||
// EMIT_MIR terminators.drop_second.built.after.mir | ||
#[custom_mir(dialect = "built")] | ||
fn drop_second<'a>(a: WriteOnDrop<'a>, b: WriteOnDrop<'a>) { |
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.
I guess we can write mem::forget without an intrinsic now 😄
@bors r+ rollup |
…-obk Support call and drop terminators in custom mir The only caveat with this change is that cleanup blocks are not supported. I would like to add them, but it's not quite clear to me what the best way to do that is, so I'll have to think about it some more. r? `@oli-obk`
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#104854 (Symlink `build/host` -> `build/$HOST_TRIPLE`) - rust-lang#105458 (Allow blocking `Command::output`) - rust-lang#105559 (bootstrap: Allow installing `llvm-tools`) - rust-lang#105789 (rustdoc: clean up margin CSS for scraped examples) - rust-lang#105792 (docs: add long error explanation for error E0320) - rust-lang#105814 (Support call and drop terminators in custom mir) - rust-lang#105829 (Speed up tidy) - rust-lang#105836 (std::fmt: Use args directly in example code) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
The only caveat with this change is that cleanup blocks are not supported. I would like to add them, but it's not quite clear to me what the best way to do that is, so I'll have to think about it some more.
r? @oli-obk