save/restore pessimistic_yield when entering bodies
#70367
Conversation
This flag is used to make the execution order around `+=` operators pessimistic. Failure to save/restore the flag was causing independent async blocks to effect one another, leading to strange ICEs and failed assumptions.
449b583 to
e7f8895
Compare
|
See #61572 for the original discussion that motivated the |
|
@Aaron1011 do you think this change makes sense? EDIT: I guess I can infer "yes" from the 👍 reaction =) |
|
@nikomatsakis: Yeah, this looks good to me. I never considered the possibility of having an async block on the RHS of the expression 😄 |
|
I don't really follow the logic here to well, but won't we also need to save/restore |
|
I don't believe we do, because when we enter into a rust/src/librustc_passes/region.rs Line 348 in 2113659 then we drain everything added since that length: rust/src/librustc_passes/region.rs Line 364 in 2113659 In other words, it's a stack, and we never look at what lies on it except the things that were pushed during a particular node. We could add an assertion that the depth of |
|
Since @Zoxc said they weren't too familiar with this, I'm going to make this r? @Aaron1011 |
|
@nikomatsakis: Could you add an additional test that has a nested use of async fn bar() {
let mut sum = 0;
sum += {
block_on(async {
baz().await;
let mut inner = 1;
inner += block_on(async {
baz().await;
0
})
})
};
} |
|
@Aaron1011 done. (Running tests locally now.) |
|
seems like tests pass |
| @@ -720,6 +720,7 @@ impl<'tcx> Visitor<'tcx> for RegionResolutionVisitor<'tcx> { | |||
| let outer_ec = mem::replace(&mut self.expr_and_pat_count, 0); | |||
| let outer_cx = self.cx; | |||
| let outer_ts = mem::take(&mut self.terminating_scopes); | |||
| let outer_pessimistic_yield = mem::replace(&mut self.pessimistic_yield, false); | |||
There was a problem hiding this comment.
Could you add a comment linking to #69307?
|
@bors r+ |
|
📌 Commit 563152d has been approved by |
|
🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened |
Rollup of 7 pull requests Successful merges: - rust-lang#67705 (Use unrolled loop for searching NULL in [u16] on Windows) - rust-lang#70367 (save/restore `pessimistic_yield` when entering bodies) - rust-lang#70822 (Don't lint for self-recursion when the function can diverge) - rust-lang#70868 (rustc_codegen_ssa: Refactor construction of linker arguments) - rust-lang#70896 (Implement Chain with Option fuses) - rust-lang#70916 (Support `#[track_caller]` on functions in `extern "Rust" { ... }`) - rust-lang#70918 (rustc_session: forbid lints override regardless of position) Failed merges: r? @ghost
This flag is used to make the execution order around
+=operatorspessimistic. Failure to save/restore the flag was causing independent
async blocks to effect one another, leading to strange ICEs and failed
assumptions.
Fixes #69307
r? @Zoxc