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 5 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
23 changes: 18 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,34 @@ 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 either must take a \
reference or ownership of outside variables they use",
);
err.help("see https://rust-lang.github.io/async-book/03_async_await/01_chapter.html#awaiting-on-a-multithreaded-executor \
Copy link
Member

Choose a reason for hiding this comment

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

I'm worried this link can go stale. It seems like using the error index would be better, we can update this link there if it ever changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this page the error index? If so, which error are you referring to?

Copy link
Contributor

Choose a reason for hiding this comment

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

That is the error index. I believe @tmandry's idea is to modify https://github.com/rust-lang/rust/blob/9e5f7d5631b8f4009ac1c693e585d4b7108d4275/compiler/rustc_error_codes/src/error_codes/E0373.md to include information from and/or a link to chapter 1 of the async book.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved the link to error index in my latest commit.

for more information");
} 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() {}
22 changes: 22 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,22 @@
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 either must take a reference or ownership of outside variables they use
sledgehammervampire marked this conversation as resolved.
Show resolved Hide resolved
= help: see https://rust-lang.github.io/async-book/03_async_await/01_chapter.html#awaiting-on-a-multithreaded-executor for more information
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`.