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

Improve async-await/generator obligation errors in some cases #70679

Merged
merged 12 commits into from
Apr 14, 2020
4 changes: 2 additions & 2 deletions src/librustc_ast_lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
await_span,
self.allow_gen_future.clone(),
);
let expr = self.lower_expr(expr);

let pinned_ident = Ident::with_dummy_span(sym::pinned);
let (pinned_pat, pinned_pat_hid) =
Expand Down Expand Up @@ -671,7 +672,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let unit = self.expr_unit(span);
let yield_expr = self.expr(
span,
hir::ExprKind::Yield(unit, hir::YieldSource::Await),
hir::ExprKind::Yield(unit, hir::YieldSource::Await { expr: Some(expr.hir_id) }),
ThinVec::new(),
);
let yield_expr = self.arena.alloc(yield_expr);
Expand Down Expand Up @@ -704,7 +705,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
// match <expr> {
// mut pinned => loop { .. }
// }
let expr = self.lower_expr(expr);
hir::ExprKind::Match(expr, arena_vec![self; pinned_arm], hir::MatchSource::AwaitDesugar)
}

Expand Down
15 changes: 12 additions & 3 deletions src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1736,15 +1736,24 @@ pub struct Destination {
#[derive(Copy, Clone, PartialEq, Eq, Debug, RustcEncodable, RustcDecodable, HashStable_Generic)]
pub enum YieldSource {
/// An `<expr>.await`.
Await,
Await { expr: Option<HirId> },
/// A plain `yield`.
Yield,
}

impl YieldSource {
pub fn is_await(&self) -> bool {
match self {
YieldSource::Await { .. } => true,
YieldSource::Yield => false,
}
}
}

impl fmt::Display for YieldSource {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(match self {
YieldSource::Await => "`await`",
YieldSource::Await { .. } => "`await`",
YieldSource::Yield => "`yield`",
})
}
Expand All @@ -1755,7 +1764,7 @@ impl From<GeneratorKind> for YieldSource {
match kind {
// Guess based on the kind of the current generator.
GeneratorKind::Gen => Self::Yield,
GeneratorKind::Async(_) => Self::Await,
GeneratorKind::Async(_) => Self::Await { expr: None },
}
}
}
Expand Down
244 changes: 162 additions & 82 deletions src/librustc_trait_selection/traits/error_reporting/suggestions.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1797,7 +1797,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// we know that the yield type must be `()`; however, the context won't contain this
// information. Hence, we check the source of the yield expression here and check its
// value's type against `()` (this check should always hold).
None if src == &hir::YieldSource::Await => {
None if src.is_await() => {
self.check_expr_coercable_to_type(&value, self.tcx.mk_unit());
self.tcx.mk_unit()
}
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/async-await/async-fn-nonsend.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ note: future is not `Send` as this value is used across an await
--> $DIR/async-fn-nonsend.rs:24:5
|
LL | let x = non_send();
| - has type `impl std::fmt::Debug`
| - has type `impl std::fmt::Debug` which is not `Send`
LL | drop(x);
LL | fut().await;
| ^^^^^^^^^^^ await occurs here, with `x` maybe used later
Expand All @@ -33,7 +33,7 @@ note: future is not `Send` as this value is used across an await
--> $DIR/async-fn-nonsend.rs:33:20
|
LL | match Some(non_send()) {
| ---------- has type `impl std::fmt::Debug`
| ---------- has type `impl std::fmt::Debug` which is not `Send`
LL | Some(_) => fut().await,
| ^^^^^^^^^^^ await occurs here, with `non_send()` maybe used later
...
Expand All @@ -54,7 +54,7 @@ note: future is not `Send` as this value is used across an await
--> $DIR/async-fn-nonsend.rs:42:9
|
LL | let f: &mut std::fmt::Formatter = panic!();
| - has type `&mut std::fmt::Formatter<'_>`
| - has type `&mut std::fmt::Formatter<'_>` which is not `Send`
LL | if non_sync().fmt(f).unwrap() == () {
LL | fut().await;
| ^^^^^^^^^^^ await occurs here, with `f` maybe used later
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/async-await/issue-64130-1-sync.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ note: future is not `Sync` as this value is used across an await
--> $DIR/issue-64130-1-sync.rs:15:5
|
LL | let x = Foo;
| - has type `Foo`
| - has type `Foo` which is not `Sync`
LL | baz().await;
| ^^^^^^^^^^^ await occurs here, with `x` maybe used later
LL | }
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/async-await/issue-64130-2-send.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ note: future is not `Send` as this value is used across an await
--> $DIR/issue-64130-2-send.rs:15:5
|
LL | let x = Foo;
| - has type `Foo`
| - has type `Foo` which is not `Send`
LL | baz().await;
| ^^^^^^^^^^^ await occurs here, with `x` maybe used later
LL | }
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/async-await/issue-64130-3-other.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ note: future does not implement `Qux` as this value is used across an await
--> $DIR/issue-64130-3-other.rs:18:5
|
LL | let x = Foo;
| - has type `Foo`
| - has type `Foo` which does not implement `Qux`
LL | baz().await;
| ^^^^^^^^^^^ await occurs here, with `x` maybe used later
LL | }
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-64130-4-async-move.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: future cannot be sent between threads safely
--> $DIR/issue-64130-4-async-move.rs:15:17
|
LL | pub fn foo() -> impl Future + Send {
| ^^^^^^^^^^^^^^^^^^ future returned by `foo` is not `Send`
| ^^^^^^^^^^^^^^^^^^ future created by async block is not `Send`
...
LL | / async move {
LL | | match client.status() {
Expand All @@ -18,7 +18,7 @@ note: future is not `Send` as this value is used across an await
--> $DIR/issue-64130-4-async-move.rs:21:26
|
LL | match client.status() {
| ------ has type `&Client`
| ------ has type `&Client` which is not `Send`
LL | 200 => {
LL | let _x = get().await;
| ^^^^^^^^^^^ await occurs here, with `client` maybe used later
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ note: future is not `Send` as this value is used across an await
--> $DIR/issue-64130-non-send-future-diags.rs:15:5
|
LL | let g = x.lock().unwrap();
| - has type `std::sync::MutexGuard<'_, u32>`
| - has type `std::sync::MutexGuard<'_, u32>` which is not `Send`
LL | baz().await;
| ^^^^^^^^^^^ await occurs here, with `g` maybe used later
LL | }
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-67252-unnamed-future.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ LL | fn spawn<T: Send>(_: T) {}
| ---- required by this bound in `spawn`
...
LL | spawn(async {
| ^^^^^ future is not `Send`
| ^^^^^ future created by async block is not `Send`
Copy link
Contributor

Choose a reason for hiding this comment

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

seems good to me

|
= help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `*mut ()`
note: future is not `Send` as this value is used across an await
--> $DIR/issue-67252-unnamed-future.rs:20:9
|
LL | let _a = std::ptr::null_mut::<()>(); // `*mut ()` is not `Send`
| -- has type `*mut ()`
| -- has type `*mut ()` which is not `Send`
LL | AFuture.await;
| ^^^^^^^^^^^^^ await occurs here, with `_a` maybe used later
LL | });
Expand Down
64 changes: 64 additions & 0 deletions src/test/ui/async-await/issue-68112.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// edition:2018

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

fn require_send(_: impl Send) {}

struct Ready<T>(Option<T>);
impl<T> Future for Ready<T> {
type Output = T;
fn poll(mut self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<T> {
Poll::Ready(self.0.take().unwrap())
}
}
fn ready<T>(t: T) -> Ready<T> {
Ready(Some(t))
}

fn make_non_send_future1() -> impl Future<Output = Arc<RefCell<i32>>> {
ready(Arc::new(RefCell::new(0)))
}

fn test1() {
let send_fut = async {
let non_send_fut = make_non_send_future1();
let _ = non_send_fut.await;
ready(0).await;
};
require_send(send_fut);
//~^ ERROR future cannot be sent between threads
}

fn test1_no_let() {
let send_fut = async {
let _ = make_non_send_future1().await;
ready(0).await;
};
require_send(send_fut);
//~^ ERROR future cannot be sent between threads
}

async fn ready2<T>(t: T) -> T { t }
fn make_non_send_future2() -> impl Future<Output = Arc<RefCell<i32>>> {
ready2(Arc::new(RefCell::new(0)))
}

// Ideally this test would have diagnostics similar to the test above, but right
// now it doesn't.
fn test2() {
let send_fut = async {
let non_send_fut = make_non_send_future2();
let _ = non_send_fut.await;
ready(0).await;
};
require_send(send_fut);
//~^ ERROR `std::cell::RefCell<i32>` cannot be shared between threads safely
}

fn main() {}
56 changes: 56 additions & 0 deletions src/test/ui/async-await/issue-68112.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
error: future cannot be sent between threads safely
--> $DIR/issue-68112.rs:34:5
|
LL | fn require_send(_: impl Send) {}
| ---- required by this bound in `require_send`
...
LL | require_send(send_fut);
| ^^^^^^^^^^^^ future created by async block is not `Send`
|
= help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<i32>`
note: future is not `Send` as it awaits another future which is not `Send`
--> $DIR/issue-68112.rs:31:17
|
LL | let _ = non_send_fut.await;
| ^^^^^^^^^^^^ await occurs here on type `impl std::future::Future`, which is not `Send`

error: future cannot be sent between threads safely
--> $DIR/issue-68112.rs:43:5
|
LL | fn require_send(_: impl Send) {}
| ---- required by this bound in `require_send`
...
LL | require_send(send_fut);
| ^^^^^^^^^^^^ future created by async block is not `Send`
|
= help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<i32>`
note: future is not `Send` as it awaits another future which is not `Send`
--> $DIR/issue-68112.rs:40:17
|
LL | let _ = make_non_send_future1().await;
| ^^^^^^^^^^^^^^^^^^^^^^^ await occurs here on type `impl std::future::Future`, which is not `Send`

error[E0277]: `std::cell::RefCell<i32>` cannot be shared between threads safely
--> $DIR/issue-68112.rs:60:5
|
LL | fn require_send(_: impl Send) {}
| ---- required by this bound in `require_send`
...
LL | require_send(send_fut);
| ^^^^^^^^^^^^ `std::cell::RefCell<i32>` cannot be shared between threads safely
|
= help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<i32>`
= note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc<std::cell::RefCell<i32>>`
= note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:47:31: 47:36 t:std::sync::Arc<std::cell::RefCell<i32>> {}]`
= note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:47:31: 47:36 t:std::sync::Arc<std::cell::RefCell<i32>> {}]>`
= note: required because it appears within the type `impl std::future::Future`
= note: required because it appears within the type `impl std::future::Future`
= note: required because it appears within the type `impl std::future::Future`
= note: required because it appears within the type `{std::future::ResumeTy, impl std::future::Future, (), i32, Ready<i32>}`
= note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:55:26: 59:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready<i32>}]`
= note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:55:26: 59:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready<i32>}]>`
= note: required because it appears within the type `impl std::future::Future`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | fn assert_send<T: Send>(_: T) {}
| ---- required by this bound in `assert_send`
...
LL | assert_send(async {
| ^^^^^^^^^^^ future returned by `main` is not `Send`
| ^^^^^^^^^^^ future created by async block is not `Send`
|
= help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `*const u8`
note: future is not `Send` as this value is used across an await
Expand All @@ -14,7 +14,7 @@ note: future is not `Send` as this value is used across an await
LL | bar(Foo(std::ptr::null())).await;
| ^^^^^^^^----------------^^^^^^^^- `std::ptr::null()` is later dropped here
| | |
| | has type `*const u8`
| | has type `*const u8` which is not `Send`
| await occurs here, with `std::ptr::null()` maybe used later
help: consider moving this into a `let` binding to create a shorter lived borrow
--> $DIR/issue-65436-raw-ptr-not-send.rs:14:13
Expand Down
56 changes: 56 additions & 0 deletions src/test/ui/generator/issue-68112.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#![feature(generators, generator_trait)]

use std::{
cell::RefCell,
sync::Arc,
pin::Pin,
ops::{Generator, GeneratorState},
};

pub struct Ready<T>(Option<T>);
impl<T> Generator<()> for Ready<T> {
type Return = T;
type Yield = ();
fn resume(mut self: Pin<&mut Self>, _args: ()) -> GeneratorState<(), T> {
GeneratorState::Complete(self.0.take().unwrap())
}
}
pub fn make_gen1<T>(t: T) -> Ready<T> {
Ready(Some(t))
}

fn require_send(_: impl Send) {}

fn make_non_send_generator() -> impl Generator<Return = Arc<RefCell<i32>>> {
make_gen1(Arc::new(RefCell::new(0)))
}

fn test1() {
let send_gen = || {
let _non_send_gen = make_non_send_generator();
yield;
};
require_send(send_gen);
//~^ ERROR generator cannot be sent between threads
}

pub fn make_gen2<T>(t: T) -> impl Generator<Return = T> {
|| {
yield;
t
}
}
fn make_non_send_generator2() -> impl Generator<Return = Arc<RefCell<i32>>> {
make_gen2(Arc::new(RefCell::new(0)))
}

fn test2() {
let send_gen = || {
let _non_send_gen = make_non_send_generator2();
yield;
};
require_send(send_gen);
//~^ ERROR `std::cell::RefCell<i32>` cannot be shared between threads safely
}

fn main() {}
Loading