Skip to content
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

Explain why borrows can't be held across yield point in async blocks #80614

Merged
merged 13 commits into from
Jan 16, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions compiler/rustc_error_codes/src/error_codes/E0373.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,24 @@ fn foo() -> Box<Fn(u32) -> u32> {

Now that the closure has its own copy of the data, there's no need to worry
about safety.

This error may also be encountered while using `async` blocks:

```compile_fail,E0373,edition2018
Copy link
Member

@tmandry tmandry Jan 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, this test passes now! It can still be simplified though:

use std::future::Future;

async fn f() {
    let v = vec![1, 2, 3i32];
    spawn(async { //~ ERROR E0373
        println!("{:?}", v)
    });
}

fn spawn<F: Future + Send + 'static>(future: F) {
    unimplemented!()
}

Otherwise this change LGTM.

Copy link
Contributor Author

@sledgehammervampire sledgehammervampire Jan 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed the code snippet to yours in my latest commit.

use std::future::Future;

async fn f() {
let v = vec![1, 2, 3i32];
spawn(async { //~ ERROR E0373
println!("{:?}", v)
});
}

fn spawn<F: Future + Send + 'static>(future: F) {
unimplemented!()
}
```

Similarly to closures, `async` blocks are not executed immediately and may
capture closed-over data by reference. For more information, see
https://rust-lang.github.io/async-book/03_async_await/01_chapter.html.
21 changes: 16 additions & 5 deletions compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1324,21 +1324,32 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
Applicability::MachineApplicable,
);

let msg = match category {
match category {
ConstraintCategory::Return(_) | ConstraintCategory::OpaqueType => {
format!("{} is returned here", kind)
let msg = format!("{} is returned here", kind);
err.span_note(constraint_span, &msg);
}
ConstraintCategory::CallArgument => {
fr_name.highlight_region_name(&mut err);
format!("function requires argument type to outlive `{}`", fr_name)
if matches!(use_span.generator_kind(), Some(generator_kind)
if matches!(generator_kind, GeneratorKind::Async(_)))
{
sledgehammervampire marked this conversation as resolved.
Show resolved Hide resolved
err.note(
"async blocks are not executed immediately and must either take a \
reference or ownership of outside variables they use",
);
} else {
let msg = format!("function requires argument type to outlive `{}`", fr_name);
err.span_note(constraint_span, &msg);
}
}
_ => bug!(
"report_escaping_closure_capture called with unexpected constraint \
category: `{:?}`",
category
),
};
err.span_note(constraint_span, &msg);
}

err
}

Expand Down
33 changes: 33 additions & 0 deletions src/test/ui/async-await/issues/issue-78938-async-block.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// edition:2018

use std::{sync::Arc, future::Future, pin::Pin, task::{Context, Poll}};

async fn f() {
let room_ref = Arc::new(Vec::new());

let gameloop_handle = spawn(async { //~ ERROR E0373
game_loop(Arc::clone(&room_ref))
});
gameloop_handle.await;
}

fn game_loop(v: Arc<Vec<usize>>) {}

fn spawn<F>(future: F) -> JoinHandle
where
F: Future + Send + 'static,
F::Output: Send + 'static,
{
loop {}
}

struct JoinHandle;

impl Future for JoinHandle {
type Output = ();
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
loop {}
}
}

fn main() {}
21 changes: 21 additions & 0 deletions src/test/ui/async-await/issues/issue-78938-async-block.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0373]: async block may outlive the current function, but it borrows `room_ref`, which is owned by the current function
--> $DIR/issue-78938-async-block.rs:8:39
|
LL | let gameloop_handle = spawn(async {
| _______________________________________^
LL | | game_loop(Arc::clone(&room_ref))
| | -------- `room_ref` is borrowed here
LL | | });
| |_____^ may outlive borrowed value `room_ref`
|
= note: async blocks are not executed immediately and must either take a reference or ownership of outside variables they use
help: to force the async block to take ownership of `room_ref` (and any other referenced variables), use the `move` keyword
|
LL | let gameloop_handle = spawn(async move {
LL | game_loop(Arc::clone(&room_ref))
LL | });
Comment on lines +12 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll need to check the suggestion that generates this becuase it seems to me we don't really need to show the whole block for this and we could make it less verbose.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps show only the line with async?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing that requires "only" to modify the following

let args_span = use_span.args_or_use();
let suggestion = match tcx.sess.source_map().span_to_snippet(args_span) {
Ok(mut string) => {
if string.starts_with("async ") {
string.insert_str(6, "move ");
} else if string.starts_with("async|") {
string.insert_str(5, " move");
} else {
string.insert_str(0, "move ");
};
string
}
Err(_) => "move |<args>| <body>".to_string(),
};

The args_span should only point at the async { and the suggested code sould only be "async move {", where now it is the whole thing.

If you wish to tackle this in this PR, it'd be great, but I don't consider it a must have yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'll leave that for later or someone else.

|

error: aborting due to previous error

For more information about this error, try `rustc --explain E0373`.