From b129b32afa3fa896f1464b9a529c30116a7a4e5c Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 24 Mar 2020 17:06:40 -0700 Subject: [PATCH 01/12] Use inner/outer generator naming instead of first/last I personally find this clearer. --- .../traits/error_reporting/suggestions.rs | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 14029f2915141..c923d9e16fe67 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -127,8 +127,8 @@ pub trait InferCtxtExt<'tcx> { scope_span: &Option, expr: Option, snippet: String, - first_generator: DefId, - last_generator: Option, + inner_generator: DefId, + outer_generator: Option, trait_ref: ty::TraitRef<'_>, target_ty: Ty<'tcx>, tables: &ty::TypeckTables<'_>, @@ -1118,8 +1118,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // - `BindingObligation` with `impl_send (Send requirement) // // The first obligation in the chain is the most useful and has the generator that captured - // the type. The last generator has information about where the bound was introduced. At - // least one generator should be present for this diagnostic to be modified. + // the type. The last generator (`outer_generator` below) has information about where the + // bound was introduced. At least one generator should be present for this diagnostic to be + // modified. let (mut trait_ref, mut target_ty) = match obligation.predicate { ty::Predicate::Trait(p, _) => { (Some(p.skip_binder().trait_ref), Some(p.skip_binder().self_ty())) @@ -1127,7 +1128,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { _ => (None, None), }; let mut generator = None; - let mut last_generator = None; + let mut outer_generator = None; let mut next_code = Some(&obligation.cause.code); while let Some(code) = next_code { debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code); @@ -1144,7 +1145,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { match ty.kind { ty::Generator(did, ..) => { generator = generator.or(Some(did)); - last_generator = Some(did); + outer_generator = Some(did); } ty::GeneratorWitness(..) => {} _ if generator.is_none() => { @@ -1248,7 +1249,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { *expr, snippet, generator_did, - last_generator, + outer_generator, trait_ref, target_ty, tables, @@ -1270,8 +1271,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { scope_span: &Option, expr: Option, snippet: String, - first_generator: DefId, - last_generator: Option, + inner_generator: DefId, + outer_generator: Option, trait_ref: ty::TraitRef<'_>, target_ty: Ty<'tcx>, tables: &ty::TypeckTables<'_>, @@ -1282,14 +1283,14 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let is_async_fn = self .tcx - .parent(first_generator) + .parent(inner_generator) .map(|parent_did| self.tcx.asyncness(parent_did)) .map(|parent_asyncness| parent_asyncness == hir::IsAsync::Async) .unwrap_or(false); let is_async_move = self .tcx .hir() - .as_local_hir_id(first_generator) + .as_local_hir_id(inner_generator) .and_then(|hir_id| self.tcx.hir().maybe_body_owned_by(hir_id)) .map(|body_id| self.tcx.hir().body(body_id)) .and_then(|body| body.generator_kind()) @@ -1318,7 +1319,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let original_span = err.span.primary_span().unwrap(); let mut span = MultiSpan::from_span(original_span); - let message = if let Some(name) = last_generator + let message = if let Some(name) = outer_generator .and_then(|generator_did| self.tcx.parent(generator_did)) .and_then(|parent_did| hir.as_local_hir_id(parent_did)) .and_then(|parent_hir_id| hir.opt_name(parent_hir_id)) From a40ec132621225f3d7e373d6630eb45f862fe39b Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 24 Mar 2020 17:23:50 -0700 Subject: [PATCH 02/12] Add test for #68112 (existing output) --- src/test/ui/async-await/issue-68112.rs | 53 ++++++++++++++++++++ src/test/ui/async-await/issue-68112.stderr | 45 +++++++++++++++++ src/test/ui/generator/issue-68112.rs | 56 ++++++++++++++++++++++ src/test/ui/generator/issue-68112.stderr | 40 ++++++++++++++++ 4 files changed, 194 insertions(+) create mode 100644 src/test/ui/async-await/issue-68112.rs create mode 100644 src/test/ui/async-await/issue-68112.stderr create mode 100644 src/test/ui/generator/issue-68112.rs create mode 100644 src/test/ui/generator/issue-68112.stderr diff --git a/src/test/ui/async-await/issue-68112.rs b/src/test/ui/async-await/issue-68112.rs new file mode 100644 index 0000000000000..0d269b1801bac --- /dev/null +++ b/src/test/ui/async-await/issue-68112.rs @@ -0,0 +1,53 @@ +// edition:2018 + +use std::{ + future::Future, + cell::RefCell, + sync::Arc, + pin::Pin, + task::{Context, Poll}, +}; + +fn require_send(_: impl Send) {} + +struct Ready(Option); +impl Future for Ready { + type Output = T; + fn poll(mut self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll { + Poll::Ready(self.0.take().unwrap()) + } +} +fn ready(t: T) -> Ready { + Ready(Some(t)) +} + +fn make_non_send_future1() -> impl Future>> { + 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 +} + +async fn ready2(t: T) -> T { t } +fn make_non_send_future2() -> impl Future>> { + ready2(Arc::new(RefCell::new(0))) +} + +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` cannot be shared between threads safely +} + +fn main() {} diff --git a/src/test/ui/async-await/issue-68112.stderr b/src/test/ui/async-await/issue-68112.stderr new file mode 100644 index 0000000000000..461967b7d8b2f --- /dev/null +++ b/src/test/ui/async-await/issue-68112.stderr @@ -0,0 +1,45 @@ +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 returned by `test1` is not `Send` + | + = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell` +note: future is not `Send` as this value is used across an await + --> $DIR/issue-68112.rs:32:9 + | +LL | let non_send_fut = make_non_send_future1(); + | ------------ has type `impl std::future::Future` +LL | let _ = non_send_fut.await; +LL | ready(0).await; + | ^^^^^^^^ await occurs here, with `non_send_fut` maybe used later +LL | }; + | - `non_send_fut` is later dropped here + +error[E0277]: `std::cell::RefCell` cannot be shared between threads safely + --> $DIR/issue-68112.rs:49:5 + | +LL | fn require_send(_: impl Send) {} + | ------------ ---- required by this bound in `require_send` +... +LL | require_send(send_fut); + | ^^^^^^^^^^^^ `std::cell::RefCell` cannot be shared between threads safely + | + = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell` + = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc>` + = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:38:31: 38:36 t:std::sync::Arc> {}]` + = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:38:31: 38:36 t:std::sync::Arc> {}]>` + = 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}` + = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:44:26: 48:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready}]` + = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:44:26: 48:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready}]>` + = note: required because it appears within the type `impl std::future::Future` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/generator/issue-68112.rs b/src/test/ui/generator/issue-68112.rs new file mode 100644 index 0000000000000..ee62cbff08f32 --- /dev/null +++ b/src/test/ui/generator/issue-68112.rs @@ -0,0 +1,56 @@ +#![feature(generators, generator_trait)] + +use std::{ + cell::RefCell, + sync::Arc, + pin::Pin, + ops::{Generator, GeneratorState}, +}; + +pub struct Ready(Option); +impl Generator<()> for Ready { + 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) -> Ready { + Ready(Some(t)) +} + +fn require_send(_: impl Send) {} + +fn make_non_send_generator() -> impl Generator>> { + 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 future cannot be sent between threads +} + +pub fn make_gen2(t: T) -> impl Generator { + || { + yield; + t + } +} +fn make_non_send_generator2() -> impl Generator>> { + 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` cannot be shared between threads safely +} + +fn main() {} diff --git a/src/test/ui/generator/issue-68112.stderr b/src/test/ui/generator/issue-68112.stderr new file mode 100644 index 0000000000000..b98afd47b566f --- /dev/null +++ b/src/test/ui/generator/issue-68112.stderr @@ -0,0 +1,40 @@ +error: future cannot be sent between threads safely + --> $DIR/issue-68112.rs:33:5 + | +LL | fn require_send(_: impl Send) {} + | ------------ ---- required by this bound in `require_send` +... +LL | require_send(send_gen); + | ^^^^^^^^^^^^ future returned by `test1` is not `Send` + | + = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell` +note: future is not `Send` as this value is used across an yield + --> $DIR/issue-68112.rs:31:9 + | +LL | let _non_send_gen = make_non_send_generator(); + | ------------- has type `impl std::ops::Generator` +LL | yield; + | ^^^^^ yield occurs here, with `_non_send_gen` maybe used later +LL | }; + | - `_non_send_gen` is later dropped here + +error[E0277]: `std::cell::RefCell` cannot be shared between threads safely + --> $DIR/issue-68112.rs:52:5 + | +LL | fn require_send(_: impl Send) {} + | ------------ ---- required by this bound in `require_send` +... +LL | require_send(send_gen); + | ^^^^^^^^^^^^ `std::cell::RefCell` cannot be shared between threads safely + | + = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell` + = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc>` + = note: required because it appears within the type `[generator@$DIR/issue-68112.rs:38:5: 41:6 t:std::sync::Arc> {()}]` + = note: required because it appears within the type `impl std::ops::Generator` + = note: required because it appears within the type `impl std::ops::Generator` + = note: required because it appears within the type `{impl std::ops::Generator, ()}` + = note: required because it appears within the type `[generator@$DIR/issue-68112.rs:48:20: 51:6 {impl std::ops::Generator, ()}]` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. From db0a5a105666ee4dffd0ab7cda9e0550836beb4c Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 24 Mar 2020 18:51:16 -0700 Subject: [PATCH 03/12] Improve span label --- .../traits/error_reporting/suggestions.rs | 28 ++++++++++++------- .../issue-64130-4-async-move.stderr | 2 +- .../issue-67252-unnamed-future.stderr | 2 +- src/test/ui/async-await/issue-68112.stderr | 2 +- .../issue-65436-raw-ptr-not-send.stderr | 2 +- src/test/ui/generator/issue-68112.stderr | 2 +- src/test/ui/generator/not-send-sync.stderr | 2 +- 7 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index c923d9e16fe67..7d43a2273fed9 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -10,7 +10,7 @@ use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; -use rustc_hir::Node; +use rustc_hir::{GeneratorKind, AsyncGeneratorKind, Node}; use rustc_middle::ty::TypeckTables; use rustc_middle::ty::{ self, AdtKind, DefIdTree, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness, @@ -1319,15 +1319,23 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let original_span = err.span.primary_span().unwrap(); let mut span = MultiSpan::from_span(original_span); - let message = if let Some(name) = outer_generator - .and_then(|generator_did| self.tcx.parent(generator_did)) - .and_then(|parent_did| hir.as_local_hir_id(parent_did)) - .and_then(|parent_hir_id| hir.opt_name(parent_hir_id)) - { - format!("future returned by `{}` is not {}", name, trait_name) - } else { - format!("future is not {}", trait_name) - }; + let message = outer_generator + .and_then(|generator_did| Some( + match self.tcx.generator_kind(generator_did).unwrap() { + GeneratorKind::Gen => format!("generator is not {}", trait_name), + GeneratorKind::Async(AsyncGeneratorKind::Fn) => + self.tcx.parent(generator_did) + .and_then(|parent_did| hir.as_local_hir_id(parent_did)) + .and_then(|parent_hir_id| hir.opt_name(parent_hir_id)) + .map(|name| format!("future returned by `{}` is not {}", + name, trait_name))?, + GeneratorKind::Async(AsyncGeneratorKind::Block) => + format!("future created by async block is not {}", trait_name), + GeneratorKind::Async(AsyncGeneratorKind::Closure) => + format!("future created by async closure is not {}", trait_name), + } + )) + .unwrap_or_else(|| format!("future is not {}", trait_name)); span.push_span_label(original_span, message); err.set_span(span); diff --git a/src/test/ui/async-await/issue-64130-4-async-move.stderr b/src/test/ui/async-await/issue-64130-4-async-move.stderr index 1e52d74f1559c..edde947764afe 100644 --- a/src/test/ui/async-await/issue-64130-4-async-move.stderr +++ b/src/test/ui/async-await/issue-64130-4-async-move.stderr @@ -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() { diff --git a/src/test/ui/async-await/issue-67252-unnamed-future.stderr b/src/test/ui/async-await/issue-67252-unnamed-future.stderr index cbcc3cf5d78a7..cec40b5510148 100644 --- a/src/test/ui/async-await/issue-67252-unnamed-future.stderr +++ b/src/test/ui/async-await/issue-67252-unnamed-future.stderr @@ -5,7 +5,7 @@ LL | fn spawn(_: T) {} | ---- required by this bound in `spawn` ... LL | spawn(async { - | ^^^^^ future 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 `*mut ()` note: future is not `Send` as this value is used across an await diff --git a/src/test/ui/async-await/issue-68112.stderr b/src/test/ui/async-await/issue-68112.stderr index 461967b7d8b2f..9f901901e207e 100644 --- a/src/test/ui/async-await/issue-68112.stderr +++ b/src/test/ui/async-await/issue-68112.stderr @@ -5,7 +5,7 @@ LL | fn require_send(_: impl Send) {} | ------------ ---- required by this bound in `require_send` ... LL | require_send(send_fut); - | ^^^^^^^^^^^^ future returned by `test1` is not `Send` + | ^^^^^^^^^^^^ future created by async block is not `Send` | = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell` note: future is not `Send` as this value is used across an await diff --git a/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr b/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr index 73e2a92d815c8..a04ae7220ec88 100644 --- a/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr +++ b/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr @@ -5,7 +5,7 @@ LL | fn assert_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 diff --git a/src/test/ui/generator/issue-68112.stderr b/src/test/ui/generator/issue-68112.stderr index b98afd47b566f..8950ff707d4aa 100644 --- a/src/test/ui/generator/issue-68112.stderr +++ b/src/test/ui/generator/issue-68112.stderr @@ -5,7 +5,7 @@ LL | fn require_send(_: impl Send) {} | ------------ ---- required by this bound in `require_send` ... LL | require_send(send_gen); - | ^^^^^^^^^^^^ future returned by `test1` is not `Send` + | ^^^^^^^^^^^^ generator is not `Send` | = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell` note: future is not `Send` as this value is used across an yield diff --git a/src/test/ui/generator/not-send-sync.stderr b/src/test/ui/generator/not-send-sync.stderr index 5f5211b5092f4..0ce9770f7aa6a 100644 --- a/src/test/ui/generator/not-send-sync.stderr +++ b/src/test/ui/generator/not-send-sync.stderr @@ -18,7 +18,7 @@ LL | fn assert_sync(_: T) {} | ---- required by this bound in `main::assert_sync` ... LL | assert_sync(|| { - | ^^^^^^^^^^^ future returned by `main` is not `Sync` + | ^^^^^^^^^^^ generator is not `Sync` | = help: within `[generator@$DIR/not-send-sync.rs:9:17: 13:6 {std::cell::Cell, ()}]`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell` note: future is not `Sync` as this value is used across an yield From 7127ff3d94fa5102bbe9b95da17d7e57bac355dd Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 24 Mar 2020 19:25:40 -0700 Subject: [PATCH 04/12] Don't annotate type when type is opaque --- .../traits/error_reporting/suggestions.rs | 7 ++++++- src/test/ui/async-await/async-fn-nonsend.stderr | 4 ++-- src/test/ui/async-await/issue-68112.stderr | 2 +- src/test/ui/generator/issue-68112.stderr | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 7d43a2273fed9..7c29315352349 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1353,7 +1353,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet), ); - span.push_span_label(target_span, format!("has type `{}`", target_ty)); + if target_ty.is_impl_trait() { + // It's not very useful to tell the user the type if it's opaque. + span.push_span_label(target_span, "created here".to_string()); + } else { + span.push_span_label(target_span, format!("has type `{}`", target_ty)); + } // If available, use the scope span to annotate the drop location. if let Some(scope_span) = scope_span { diff --git a/src/test/ui/async-await/async-fn-nonsend.stderr b/src/test/ui/async-await/async-fn-nonsend.stderr index 04df6203e43f1..176c62fcd7d92 100644 --- a/src/test/ui/async-await/async-fn-nonsend.stderr +++ b/src/test/ui/async-await/async-fn-nonsend.stderr @@ -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` + | - created here LL | drop(x); LL | fut().await; | ^^^^^^^^^^^ await occurs here, with `x` maybe used later @@ -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` + | ---------- created here LL | Some(_) => fut().await, | ^^^^^^^^^^^ await occurs here, with `non_send()` maybe used later ... diff --git a/src/test/ui/async-await/issue-68112.stderr b/src/test/ui/async-await/issue-68112.stderr index 9f901901e207e..eec2171024ac4 100644 --- a/src/test/ui/async-await/issue-68112.stderr +++ b/src/test/ui/async-await/issue-68112.stderr @@ -12,7 +12,7 @@ note: future is not `Send` as this value is used across an await --> $DIR/issue-68112.rs:32:9 | LL | let non_send_fut = make_non_send_future1(); - | ------------ has type `impl std::future::Future` + | ------------ created here LL | let _ = non_send_fut.await; LL | ready(0).await; | ^^^^^^^^ await occurs here, with `non_send_fut` maybe used later diff --git a/src/test/ui/generator/issue-68112.stderr b/src/test/ui/generator/issue-68112.stderr index 8950ff707d4aa..273fec082cf8d 100644 --- a/src/test/ui/generator/issue-68112.stderr +++ b/src/test/ui/generator/issue-68112.stderr @@ -12,7 +12,7 @@ note: future is not `Send` as this value is used across an yield --> $DIR/issue-68112.rs:31:9 | LL | let _non_send_gen = make_non_send_generator(); - | ------------- has type `impl std::ops::Generator` + | ------------- created here LL | yield; | ^^^^^ yield occurs here, with `_non_send_gen` maybe used later LL | }; From 6edfd66c5db78a4672e054c7a9a8207da64e98f6 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 24 Mar 2020 21:13:05 -0700 Subject: [PATCH 05/12] Use "generator" instead of "future" when appropriate --- .../traits/error_reporting/suggestions.rs | 29 +++++++++---------- src/test/ui/generator/issue-68112.rs | 2 +- src/test/ui/generator/issue-68112.stderr | 4 +-- src/test/ui/generator/not-send-sync.rs | 2 +- src/test/ui/generator/not-send-sync.stderr | 4 +-- 5 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 7c29315352349..0f4fbc41d1611 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1281,13 +1281,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ) { let source_map = self.tcx.sess.source_map(); - let is_async_fn = self - .tcx - .parent(inner_generator) - .map(|parent_did| self.tcx.asyncness(parent_did)) - .map(|parent_asyncness| parent_asyncness == hir::IsAsync::Async) - .unwrap_or(false); - let is_async_move = self + let is_async = self .tcx .hir() .as_local_hir_id(inner_generator) @@ -1299,7 +1293,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { _ => false, }) .unwrap_or(false); - let await_or_yield = if is_async_fn || is_async_move { "await" } else { "yield" }; + let await_or_yield = if is_async { "await" } else { "yield" }; + let future_or_generator = if is_async { "future" } else { "generator" }; // Special case the primary error message when send or sync is the trait that was // not implemented. @@ -1312,7 +1307,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err.clear_code(); err.set_primary_message(format!( - "future cannot be {} between threads safely", + "{} cannot be {} between threads safely", + future_or_generator, trait_verb )); @@ -1335,14 +1331,18 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { format!("future created by async closure is not {}", trait_name), } )) - .unwrap_or_else(|| format!("future is not {}", trait_name)); + .unwrap_or_else(|| format!("{} is not {}", future_or_generator, trait_name)); span.push_span_label(original_span, message); err.set_span(span); - format!("is not {}", trait_name) + format!("{} is not {}", future_or_generator, trait_name) } else { - format!("does not implement `{}`", trait_ref.print_only_trait_path()) + format!( + "{} does not implement `{}`", + future_or_generator, + trait_ref.print_only_trait_path() + ) }; // Look at the last interior type to get a span for the `.await`. @@ -1370,10 +1370,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err.span_note( span, - &format!( - "future {} as this value is used across an {}", - trait_explanation, await_or_yield, - ), + &format!("{} as this value is used across an {}", trait_explanation, await_or_yield), ); if let Some(expr_id) = expr { diff --git a/src/test/ui/generator/issue-68112.rs b/src/test/ui/generator/issue-68112.rs index ee62cbff08f32..9ab2abf740572 100644 --- a/src/test/ui/generator/issue-68112.rs +++ b/src/test/ui/generator/issue-68112.rs @@ -31,7 +31,7 @@ fn test1() { yield; }; require_send(send_gen); - //~^ ERROR future cannot be sent between threads + //~^ ERROR generator cannot be sent between threads } pub fn make_gen2(t: T) -> impl Generator { diff --git a/src/test/ui/generator/issue-68112.stderr b/src/test/ui/generator/issue-68112.stderr index 273fec082cf8d..f40771d2826d6 100644 --- a/src/test/ui/generator/issue-68112.stderr +++ b/src/test/ui/generator/issue-68112.stderr @@ -1,4 +1,4 @@ -error: future cannot be sent between threads safely +error: generator cannot be sent between threads safely --> $DIR/issue-68112.rs:33:5 | LL | fn require_send(_: impl Send) {} @@ -8,7 +8,7 @@ LL | require_send(send_gen); | ^^^^^^^^^^^^ generator is not `Send` | = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell` -note: future is not `Send` as this value is used across an yield +note: generator is not `Send` as this value is used across an yield --> $DIR/issue-68112.rs:31:9 | LL | let _non_send_gen = make_non_send_generator(); diff --git a/src/test/ui/generator/not-send-sync.rs b/src/test/ui/generator/not-send-sync.rs index 0db01c6f756ac..8ca5565fb2ab5 100644 --- a/src/test/ui/generator/not-send-sync.rs +++ b/src/test/ui/generator/not-send-sync.rs @@ -7,7 +7,7 @@ fn main() { fn assert_send(_: T) {} assert_sync(|| { - //~^ ERROR: future cannot be shared between threads safely + //~^ ERROR: generator cannot be shared between threads safely let a = Cell::new(2); yield; }); diff --git a/src/test/ui/generator/not-send-sync.stderr b/src/test/ui/generator/not-send-sync.stderr index 0ce9770f7aa6a..fb59ef5f433a1 100644 --- a/src/test/ui/generator/not-send-sync.stderr +++ b/src/test/ui/generator/not-send-sync.stderr @@ -11,7 +11,7 @@ LL | assert_send(|| { = note: required because of the requirements on the impl of `std::marker::Send` for `&std::cell::Cell` = note: required because it appears within the type `[generator@$DIR/not-send-sync.rs:16:17: 20:6 a:&std::cell::Cell _]` -error: future cannot be shared between threads safely +error: generator cannot be shared between threads safely --> $DIR/not-send-sync.rs:9:5 | LL | fn assert_sync(_: T) {} @@ -21,7 +21,7 @@ LL | assert_sync(|| { | ^^^^^^^^^^^ generator is not `Sync` | = help: within `[generator@$DIR/not-send-sync.rs:9:17: 13:6 {std::cell::Cell, ()}]`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell` -note: future is not `Sync` as this value is used across an yield +note: generator is not `Sync` as this value is used across an yield --> $DIR/not-send-sync.rs:12:9 | LL | let a = Cell::new(2); From aed7c30e4f794bc285aeca36dd9e2cc02cef2754 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 1 Apr 2020 18:53:00 -0700 Subject: [PATCH 06/12] Use clearer message when obligation is caused by await expr --- src/librustc_ast_lowering/expr.rs | 4 +- src/librustc_hir/hir.rs | 15 +- .../traits/error_reporting/suggestions.rs | 142 +++++++++++++----- src/librustc_typeck/check/expr.rs | 2 +- src/test/ui/async-await/issue-68112.stderr | 9 +- 5 files changed, 126 insertions(+), 46 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 9984eb4e2825f..2fc58efbf0e7f 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -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) = @@ -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); @@ -704,7 +705,6 @@ impl<'hir> LoweringContext<'_, 'hir> { // match { // mut pinned => loop { .. } // } - let expr = self.lower_expr(expr); hir::ExprKind::Match(expr, arena_vec![self; pinned_arm], hir::MatchSource::AwaitDesugar) } diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index b719d576d6f67..440fdc662862e 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -1736,15 +1736,24 @@ pub struct Destination { #[derive(Copy, Clone, PartialEq, Eq, Debug, RustcEncodable, RustcDecodable, HashStable_Generic)] pub enum YieldSource { /// An `.await`. - Await, + Await { expr: Option }, /// 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`", }) } @@ -1755,7 +1764,7 @@ impl From 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 }, } } } diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 0f4fbc41d1611..f81992e19c967 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -127,13 +127,14 @@ pub trait InferCtxtExt<'tcx> { scope_span: &Option, expr: Option, snippet: String, - inner_generator: DefId, + inner_generator_body: Option<&hir::Body<'_>>, outer_generator: Option, trait_ref: ty::TraitRef<'_>, target_ty: Ty<'tcx>, tables: &ty::TypeckTables<'_>, obligation: &PredicateObligation<'tcx>, next_code: Option<&ObligationCauseCode<'tcx>>, + from_awaited_ty: Option, ); fn note_obligation_cause_code( @@ -1203,6 +1204,17 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } }; + let generator_body = self.tcx + .hir() + .as_local_hir_id(generator_did) + .and_then(|hir_id| self.tcx.hir().maybe_body_owned_by(hir_id)) + .map(|body_id| self.tcx.hir().body(body_id)); + let mut visitor = AwaitsVisitor::default(); + if let Some(body) = generator_body { + visitor.visit_body(body); + } + debug!("maybe_note_obligation_cause_for_async_await: awaits = {:?}", visitor.awaits); + // Look for a type inside the generator interior that matches the target type to get // a span. let target_ty_erased = self.tcx.erase_regions(&target_ty); @@ -1232,8 +1244,26 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ); eq }) - .map(|ty::GeneratorInteriorTypeCause { span, scope_span, expr, .. }| { - (span, source_map.span_to_snippet(*span), scope_span, expr) + .map(|cause| { + // Check to see if any awaited expressions have the target type. + let from_awaited_ty = visitor.awaits.into_iter() + .map(|id| self.tcx.hir().expect_expr(id)) + .find(|expr| { + let ty = tables.expr_ty_adjusted(&expr); + // Compare types using the same logic as above. + let ty_erased = self.tcx.erase_late_bound_regions(&ty::Binder::bind(ty)); + let ty_erased = self.tcx.erase_regions(&ty_erased); + let eq = ty::TyS::same_type(ty_erased, target_ty_erased); + debug!( + "maybe_note_obligation_cause_for_async_await: await_expr={:?} \ + await_ty_erased={:?} target_ty_erased={:?} eq={:?}", + expr, ty_erased, target_ty_erased, eq + ); + eq + }) + .map(|expr| expr.span); + let ty::GeneratorInteriorTypeCause { span, scope_span, expr, .. } = cause; + (span, source_map.span_to_snippet(*span), scope_span, expr, from_awaited_ty) }); debug!( @@ -1241,20 +1271,21 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { generator_interior_types={:?} target_span={:?}", target_ty, tables.generator_interior_types, target_span ); - if let Some((target_span, Ok(snippet), scope_span, expr)) = target_span { + if let Some((target_span, Ok(snippet), scope_span, expr, from_awaited_ty)) = target_span { self.note_obligation_cause_for_async_await( err, *target_span, scope_span, *expr, snippet, - generator_did, + generator_body, outer_generator, trait_ref, target_ty, tables, obligation, next_code, + from_awaited_ty, ); true } else { @@ -1271,22 +1302,18 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { scope_span: &Option, expr: Option, snippet: String, - inner_generator: DefId, + inner_generator_body: Option<&hir::Body<'_>>, outer_generator: Option, trait_ref: ty::TraitRef<'_>, target_ty: Ty<'tcx>, tables: &ty::TypeckTables<'_>, obligation: &PredicateObligation<'tcx>, next_code: Option<&ObligationCauseCode<'tcx>>, + from_awaited_ty: Option, ) { let source_map = self.tcx.sess.source_map(); - let is_async = self - .tcx - .hir() - .as_local_hir_id(inner_generator) - .and_then(|hir_id| self.tcx.hir().maybe_body_owned_by(hir_id)) - .map(|body_id| self.tcx.hir().body(body_id)) + let is_async = inner_generator_body .and_then(|body| body.generator_kind()) .map(|generator_kind| match generator_kind { hir::GeneratorKind::Async(..) => true, @@ -1345,33 +1372,57 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ) }; - // Look at the last interior type to get a span for the `.await`. - let await_span = tables.generator_interior_types.iter().map(|t| t.span).last().unwrap(); - let mut span = MultiSpan::from_span(await_span); - span.push_span_label( - await_span, - format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet), - ); + let push_target_span = |span: &mut MultiSpan| { + if target_ty.is_impl_trait() { + // It's not very useful to tell the user the type if it's opaque. + span.push_span_label(target_span, "created here".to_string()); + } else { + span.push_span_label(target_span, format!("has type `{}`", target_ty)); + } + }; - if target_ty.is_impl_trait() { - // It's not very useful to tell the user the type if it's opaque. - span.push_span_label(target_span, "created here".to_string()); - } else { - span.push_span_label(target_span, format!("has type `{}`", target_ty)); - } + if let Some(await_span) = from_awaited_ty { + // The type causing this obligation is one being awaited at await_span. + let mut span = MultiSpan::from_span(await_span); + span.push_span_label( + await_span, + "await occurs here".to_string(), + ); + + push_target_span(&mut span); - // If available, use the scope span to annotate the drop location. - if let Some(scope_span) = scope_span { + err.span_note( + span, + &format!("{} as this value is used in an await", trait_explanation), + ); + } else { + // Look at the last interior type to get a span for the `.await`. + debug!( + "note_obligation_cause_for_async_await generator_interior_types: {:#?}", + tables.generator_interior_types + ); + let await_span = tables.generator_interior_types.iter().map(|t| t.span).last().unwrap(); + let mut span = MultiSpan::from_span(await_span); span.push_span_label( - source_map.end_point(*scope_span), - format!("`{}` is later dropped here", snippet), + await_span, + format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet), ); - } - err.span_note( - span, - &format!("{} as this value is used across an {}", trait_explanation, await_or_yield), - ); + push_target_span(&mut span); + + // If available, use the scope span to annotate the drop location. + if let Some(scope_span) = scope_span { + span.push_span_label( + source_map.end_point(*scope_span), + format!("`{}` is later dropped here", snippet), + ); + } + + err.span_note( + span, + &format!("{} as this value is used across an {}", trait_explanation, await_or_yield), + ); + } if let Some(expr_id) = expr { let expr = hir.expect_expr(expr_id); @@ -1716,6 +1767,29 @@ impl<'v> Visitor<'v> for ReturnsVisitor<'v> { } } +/// Collect all the awaited expressions within the input expression. +#[derive(Default)] +struct AwaitsVisitor { + awaits: Vec, +} + +impl<'v> Visitor<'v> for AwaitsVisitor { + type Map = hir::intravisit::ErasedMap<'v>; + + fn nested_visit_map(&mut self) -> hir::intravisit::NestedVisitorMap { + hir::intravisit::NestedVisitorMap::None + } + + fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { + match ex.kind { + hir::ExprKind::Yield(_, hir::YieldSource::Await { expr: Some(id) }) => + self.awaits.push(id), + _ => (), + } + hir::intravisit::walk_expr(self, ex) + } +} + pub trait NextTypeParamName { fn next_type_param_name(&self, name: Option<&str>) -> String; } diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 57e2349bb2e80..7cb51b4d6d833 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -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() } diff --git a/src/test/ui/async-await/issue-68112.stderr b/src/test/ui/async-await/issue-68112.stderr index eec2171024ac4..f79908110c4d4 100644 --- a/src/test/ui/async-await/issue-68112.stderr +++ b/src/test/ui/async-await/issue-68112.stderr @@ -8,16 +8,13 @@ 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` -note: future is not `Send` as this value is used across an await - --> $DIR/issue-68112.rs:32:9 +note: future is not `Send` as this value is used in an await + --> $DIR/issue-68112.rs:31:17 | LL | let non_send_fut = make_non_send_future1(); | ------------ created here LL | let _ = non_send_fut.await; -LL | ready(0).await; - | ^^^^^^^^ await occurs here, with `non_send_fut` maybe used later -LL | }; - | - `non_send_fut` is later dropped here + | ^^^^^^^^^^^^ await occurs here error[E0277]: `std::cell::RefCell` cannot be shared between threads safely --> $DIR/issue-68112.rs:49:5 From e340375ef8eb551bae29f36bf562aab1162939db Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 1 Apr 2020 19:04:55 -0700 Subject: [PATCH 07/12] Don't double-annotate the same Span --- .../traits/error_reporting/suggestions.rs | 4 ++- src/test/ui/async-await/issue-68112.rs | 9 ++++++ src/test/ui/async-await/issue-68112.stderr | 28 +++++++++++++++---- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index f81992e19c967..aa0add21db956 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1389,7 +1389,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { "await occurs here".to_string(), ); - push_target_span(&mut span); + if target_span != await_span { + push_target_span(&mut span); + } err.span_note( span, diff --git a/src/test/ui/async-await/issue-68112.rs b/src/test/ui/async-await/issue-68112.rs index 0d269b1801bac..604cc51921e0e 100644 --- a/src/test/ui/async-await/issue-68112.rs +++ b/src/test/ui/async-await/issue-68112.rs @@ -35,6 +35,15 @@ fn test1() { //~^ 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 } fn make_non_send_future2() -> impl Future>> { ready2(Arc::new(RefCell::new(0))) diff --git a/src/test/ui/async-await/issue-68112.stderr b/src/test/ui/async-await/issue-68112.stderr index f79908110c4d4..c0659bf944b81 100644 --- a/src/test/ui/async-await/issue-68112.stderr +++ b/src/test/ui/async-await/issue-68112.stderr @@ -16,8 +16,24 @@ LL | let non_send_fut = make_non_send_future1(); LL | let _ = non_send_fut.await; | ^^^^^^^^^^^^ await occurs here +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` +note: future is not `Send` as this value is used in an await + --> $DIR/issue-68112.rs:40:17 + | +LL | let _ = make_non_send_future1().await; + | ^^^^^^^^^^^^^^^^^^^^^^^ await occurs here + error[E0277]: `std::cell::RefCell` cannot be shared between threads safely - --> $DIR/issue-68112.rs:49:5 + --> $DIR/issue-68112.rs:58:5 | LL | fn require_send(_: impl Send) {} | ------------ ---- required by this bound in `require_send` @@ -27,16 +43,16 @@ LL | require_send(send_fut); | = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell` = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc>` - = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:38:31: 38:36 t:std::sync::Arc> {}]` - = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:38:31: 38:36 t:std::sync::Arc> {}]>` + = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:47:31: 47:36 t:std::sync::Arc> {}]` + = 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> {}]>` = 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}` - = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:44:26: 48:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready}]` - = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:44:26: 48:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready}]>` + = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:53:26: 57:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready}]` + = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:53:26: 57:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready}]>` = note: required because it appears within the type `impl std::future::Future` -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0277`. From 8ce334d53c91c5b9c5b4f809df7f81fbf95c2445 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 1 Apr 2020 19:14:51 -0700 Subject: [PATCH 08/12] rustfmt --- .../traits/error_reporting/suggestions.rs | 59 +++++++++++-------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index aa0add21db956..bbe059dda6f24 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -10,7 +10,7 @@ use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; -use rustc_hir::{GeneratorKind, AsyncGeneratorKind, Node}; +use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Node}; use rustc_middle::ty::TypeckTables; use rustc_middle::ty::{ self, AdtKind, DefIdTree, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness, @@ -1204,7 +1204,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } }; - let generator_body = self.tcx + let generator_body = self + .tcx .hir() .as_local_hir_id(generator_did) .and_then(|hir_id| self.tcx.hir().maybe_body_owned_by(hir_id)) @@ -1246,7 +1247,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { }) .map(|cause| { // Check to see if any awaited expressions have the target type. - let from_awaited_ty = visitor.awaits.into_iter() + let from_awaited_ty = visitor + .awaits + .into_iter() .map(|id| self.tcx.hir().expect_expr(id)) .find(|expr| { let ty = tables.expr_ty_adjusted(&expr); @@ -1335,29 +1338,32 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err.clear_code(); err.set_primary_message(format!( "{} cannot be {} between threads safely", - future_or_generator, - trait_verb + future_or_generator, trait_verb )); let original_span = err.span.primary_span().unwrap(); let mut span = MultiSpan::from_span(original_span); let message = outer_generator - .and_then(|generator_did| Some( - match self.tcx.generator_kind(generator_did).unwrap() { + .and_then(|generator_did| { + Some(match self.tcx.generator_kind(generator_did).unwrap() { GeneratorKind::Gen => format!("generator is not {}", trait_name), - GeneratorKind::Async(AsyncGeneratorKind::Fn) => - self.tcx.parent(generator_did) - .and_then(|parent_did| hir.as_local_hir_id(parent_did)) - .and_then(|parent_hir_id| hir.opt_name(parent_hir_id)) - .map(|name| format!("future returned by `{}` is not {}", - name, trait_name))?, - GeneratorKind::Async(AsyncGeneratorKind::Block) => - format!("future created by async block is not {}", trait_name), - GeneratorKind::Async(AsyncGeneratorKind::Closure) => - format!("future created by async closure is not {}", trait_name), - } - )) + GeneratorKind::Async(AsyncGeneratorKind::Fn) => self + .tcx + .parent(generator_did) + .and_then(|parent_did| hir.as_local_hir_id(parent_did)) + .and_then(|parent_hir_id| hir.opt_name(parent_hir_id)) + .map(|name| { + format!("future returned by `{}` is not {}", name, trait_name) + })?, + GeneratorKind::Async(AsyncGeneratorKind::Block) => { + format!("future created by async block is not {}", trait_name) + } + GeneratorKind::Async(AsyncGeneratorKind::Closure) => { + format!("future created by async closure is not {}", trait_name) + } + }) + }) .unwrap_or_else(|| format!("{} is not {}", future_or_generator, trait_name)); span.push_span_label(original_span, message); @@ -1384,10 +1390,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { if let Some(await_span) = from_awaited_ty { // The type causing this obligation is one being awaited at await_span. let mut span = MultiSpan::from_span(await_span); - span.push_span_label( - await_span, - "await occurs here".to_string(), - ); + span.push_span_label(await_span, "await occurs here".to_string()); if target_span != await_span { push_target_span(&mut span); @@ -1422,7 +1425,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err.span_note( span, - &format!("{} as this value is used across an {}", trait_explanation, await_or_yield), + &format!( + "{} as this value is used across an {}", + trait_explanation, await_or_yield + ), ); } @@ -1784,8 +1790,9 @@ impl<'v> Visitor<'v> for AwaitsVisitor { fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { match ex.kind { - hir::ExprKind::Yield(_, hir::YieldSource::Await { expr: Some(id) }) => - self.awaits.push(id), + hir::ExprKind::Yield(_, hir::YieldSource::Await { expr: Some(id) }) => { + self.awaits.push(id) + } _ => (), } hir::intravisit::walk_expr(self, ex) From df0a16b29fcd4e785598b4205a7b9a8cd4c48171 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 1 Apr 2020 19:54:11 -0700 Subject: [PATCH 09/12] Include type info when available for awaited expr --- .../traits/error_reporting/suggestions.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index bbe059dda6f24..d458104560ba8 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1378,10 +1378,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ) }; - let push_target_span = |span: &mut MultiSpan| { + let push_target_span_with_fallback = |span: &mut MultiSpan, fallback: &str| { if target_ty.is_impl_trait() { // It's not very useful to tell the user the type if it's opaque. - span.push_span_label(target_span, "created here".to_string()); + span.push_span_label(target_span, fallback.to_string()); } else { span.push_span_label(target_span, format!("has type `{}`", target_ty)); } @@ -1390,10 +1390,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { if let Some(await_span) = from_awaited_ty { // The type causing this obligation is one being awaited at await_span. let mut span = MultiSpan::from_span(await_span); - span.push_span_label(await_span, "await occurs here".to_string()); - if target_span != await_span { - push_target_span(&mut span); + if target_span == await_span { + push_target_span_with_fallback(&mut span, "await occurs here"); + } else { + span.push_span_label(await_span, "await occurs here".to_string()); + push_target_span_with_fallback(&mut span, "created here"); } err.span_note( @@ -1413,7 +1415,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet), ); - push_target_span(&mut span); + push_target_span_with_fallback(&mut span, "created here"); // If available, use the scope span to annotate the drop location. if let Some(scope_span) = scope_span { From 00795a99401865e6ade63600619eb5ee5a34e7c6 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 7 Apr 2020 18:50:27 -0700 Subject: [PATCH 10/12] Fix style nits --- .../traits/error_reporting/suggestions.rs | 82 +++++++++---------- 1 file changed, 37 insertions(+), 45 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index d458104560ba8..99e91bbe089c7 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1095,6 +1095,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { obligation.predicate, obligation.cause.span ); let source_map = self.tcx.sess.source_map(); + let hir = self.tcx.hir(); // Attempt to detect an async-await error by looking at the obligation causes, looking // for a generator to be present. @@ -1178,7 +1179,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let span = self.tcx.def_span(generator_did); // Do not ICE on closure typeck (#66868). - if self.tcx.hir().as_local_hir_id(generator_did).is_none() { + if hir.as_local_hir_id(generator_did).is_none() { return false; } @@ -1204,12 +1205,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } }; - let generator_body = self - .tcx - .hir() + let generator_body = hir .as_local_hir_id(generator_did) - .and_then(|hir_id| self.tcx.hir().maybe_body_owned_by(hir_id)) - .map(|body_id| self.tcx.hir().body(body_id)); + .and_then(|hir_id| hir.maybe_body_owned_by(hir_id)) + .map(|body_id| hir.body(body_id)); let mut visitor = AwaitsVisitor::default(); if let Some(body) = generator_body { visitor.visit_body(body); @@ -1219,50 +1218,46 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // Look for a type inside the generator interior that matches the target type to get // a span. let target_ty_erased = self.tcx.erase_regions(&target_ty); + let ty_matches = |ty| -> bool { + // Careful: the regions for types that appear in the + // generator interior are not generally known, so we + // want to erase them when comparing (and anyway, + // `Send` and other bounds are generally unaffected by + // the choice of region). When erasing regions, we + // also have to erase late-bound regions. This is + // because the types that appear in the generator + // interior generally contain "bound regions" to + // represent regions that are part of the suspended + // generator frame. Bound regions are preserved by + // `erase_regions` and so we must also call + // `erase_late_bound_regions`. + let ty_erased = self.tcx.erase_late_bound_regions(&ty::Binder::bind(ty)); + let ty_erased = self.tcx.erase_regions(&ty_erased); + let eq = ty::TyS::same_type(ty_erased, target_ty_erased); + debug!( + "maybe_note_obligation_cause_for_async_await: ty_erased={:?} \ + target_ty_erased={:?} eq={:?}", + ty_erased, target_ty_erased, eq + ); + eq + }; let target_span = tables .generator_interior_types .iter() - .find(|ty::GeneratorInteriorTypeCause { ty, .. }| { - // Careful: the regions for types that appear in the - // generator interior are not generally known, so we - // want to erase them when comparing (and anyway, - // `Send` and other bounds are generally unaffected by - // the choice of region). When erasing regions, we - // also have to erase late-bound regions. This is - // because the types that appear in the generator - // interior generally contain "bound regions" to - // represent regions that are part of the suspended - // generator frame. Bound regions are preserved by - // `erase_regions` and so we must also call - // `erase_late_bound_regions`. - let ty_erased = self.tcx.erase_late_bound_regions(&ty::Binder::bind(*ty)); - let ty_erased = self.tcx.erase_regions(&ty_erased); - let eq = ty::TyS::same_type(ty_erased, target_ty_erased); - debug!( - "maybe_note_obligation_cause_for_async_await: ty_erased={:?} \ - target_ty_erased={:?} eq={:?}", - ty_erased, target_ty_erased, eq - ); - eq - }) + .find(|ty::GeneratorInteriorTypeCause { ty, .. }| ty_matches(ty)) .map(|cause| { // Check to see if any awaited expressions have the target type. let from_awaited_ty = visitor .awaits .into_iter() - .map(|id| self.tcx.hir().expect_expr(id)) - .find(|expr| { - let ty = tables.expr_ty_adjusted(&expr); - // Compare types using the same logic as above. - let ty_erased = self.tcx.erase_late_bound_regions(&ty::Binder::bind(ty)); - let ty_erased = self.tcx.erase_regions(&ty_erased); - let eq = ty::TyS::same_type(ty_erased, target_ty_erased); + .map(|id| hir.expect_expr(id)) + .find(|await_expr| { + let ty = tables.expr_ty_adjusted(&await_expr); debug!( - "maybe_note_obligation_cause_for_async_await: await_expr={:?} \ - await_ty_erased={:?} target_ty_erased={:?} eq={:?}", - expr, ty_erased, target_ty_erased, eq + "maybe_note_obligation_cause_for_async_await: await_expr={:?}", + await_expr ); - eq + ty_matches(ty) }) .map(|expr| expr.span); let ty::GeneratorInteriorTypeCause { span, scope_span, expr, .. } = cause; @@ -1791,11 +1786,8 @@ impl<'v> Visitor<'v> for AwaitsVisitor { } fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { - match ex.kind { - hir::ExprKind::Yield(_, hir::YieldSource::Await { expr: Some(id) }) => { - self.awaits.push(id) - } - _ => (), + if let hir::ExprKind::Yield(_, hir::YieldSource::Await { expr: Some(id) }) = ex.kind { + self.awaits.push(id) } hir::intravisit::walk_expr(self, ex) } From df64c5d260d3841ded8affe68f1be6e47ed1fb26 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 8 Apr 2020 11:54:31 -0700 Subject: [PATCH 11/12] Incorporate feedback into diagnostics --- .../traits/error_reporting/suggestions.rs | 44 ++++++++----------- .../ui/async-await/async-fn-nonsend.stderr | 6 +-- .../ui/async-await/issue-64130-1-sync.stderr | 2 +- .../ui/async-await/issue-64130-2-send.stderr | 2 +- .../ui/async-await/issue-64130-3-other.stderr | 2 +- .../issue-64130-4-async-move.stderr | 2 +- .../issue-64130-non-send-future-diags.stderr | 2 +- .../issue-67252-unnamed-future.stderr | 2 +- src/test/ui/async-await/issue-68112.rs | 2 + src/test/ui/async-await/issue-68112.stderr | 16 +++---- .../issue-65436-raw-ptr-not-send.stderr | 2 +- src/test/ui/generator/issue-68112.stderr | 4 +- src/test/ui/generator/not-send-sync.stderr | 4 +- 13 files changed, 41 insertions(+), 49 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 99e91bbe089c7..77ecdd08f78e0 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1318,7 +1318,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { _ => false, }) .unwrap_or(false); - let await_or_yield = if is_async { "await" } else { "yield" }; + let (await_or_yield, an_await_or_yield) = + if is_async { ("await", "an await") } else { ("yield", "a yield") }; let future_or_generator = if is_async { "future" } else { "generator" }; // Special case the primary error message when send or sync is the trait that was @@ -1364,38 +1365,26 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { span.push_span_label(original_span, message); err.set_span(span); - format!("{} is not {}", future_or_generator, trait_name) + format!("is not {}", trait_name) } else { - format!( - "{} does not implement `{}`", - future_or_generator, - trait_ref.print_only_trait_path() - ) - }; - - let push_target_span_with_fallback = |span: &mut MultiSpan, fallback: &str| { - if target_ty.is_impl_trait() { - // It's not very useful to tell the user the type if it's opaque. - span.push_span_label(target_span, fallback.to_string()); - } else { - span.push_span_label(target_span, format!("has type `{}`", target_ty)); - } + format!("does not implement `{}`", trait_ref.print_only_trait_path()) }; if let Some(await_span) = from_awaited_ty { // The type causing this obligation is one being awaited at await_span. let mut span = MultiSpan::from_span(await_span); - if target_span == await_span { - push_target_span_with_fallback(&mut span, "await occurs here"); - } else { - span.push_span_label(await_span, "await occurs here".to_string()); - push_target_span_with_fallback(&mut span, "created here"); - } + span.push_span_label( + await_span, + format!("await occurs here on type `{}`, which {}", target_ty, trait_explanation), + ); err.span_note( span, - &format!("{} as this value is used in an await", trait_explanation), + &format!( + "future {not_trait} as it awaits another future which {not_trait}", + not_trait = trait_explanation + ), ); } else { // Look at the last interior type to get a span for the `.await`. @@ -1410,7 +1399,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet), ); - push_target_span_with_fallback(&mut span, "created here"); + span.push_span_label( + target_span, + format!("has type `{}` which {}", target_ty, trait_explanation), + ); // If available, use the scope span to annotate the drop location. if let Some(scope_span) = scope_span { @@ -1423,8 +1415,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err.span_note( span, &format!( - "{} as this value is used across an {}", - trait_explanation, await_or_yield + "{} {} as this value is used across {}", + future_or_generator, trait_explanation, an_await_or_yield ), ); } diff --git a/src/test/ui/async-await/async-fn-nonsend.stderr b/src/test/ui/async-await/async-fn-nonsend.stderr index 176c62fcd7d92..d36d59f1f68f6 100644 --- a/src/test/ui/async-await/async-fn-nonsend.stderr +++ b/src/test/ui/async-await/async-fn-nonsend.stderr @@ -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(); - | - created here + | - has type `impl std::fmt::Debug` which is not `Send` LL | drop(x); LL | fut().await; | ^^^^^^^^^^^ await occurs here, with `x` maybe used later @@ -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()) { - | ---------- created here + | ---------- has type `impl std::fmt::Debug` which is not `Send` LL | Some(_) => fut().await, | ^^^^^^^^^^^ await occurs here, with `non_send()` maybe used later ... @@ -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 diff --git a/src/test/ui/async-await/issue-64130-1-sync.stderr b/src/test/ui/async-await/issue-64130-1-sync.stderr index b7a88c10e7411..42e9e4642cea5 100644 --- a/src/test/ui/async-await/issue-64130-1-sync.stderr +++ b/src/test/ui/async-await/issue-64130-1-sync.stderr @@ -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 | } diff --git a/src/test/ui/async-await/issue-64130-2-send.stderr b/src/test/ui/async-await/issue-64130-2-send.stderr index ec18308877177..f6f834618d36f 100644 --- a/src/test/ui/async-await/issue-64130-2-send.stderr +++ b/src/test/ui/async-await/issue-64130-2-send.stderr @@ -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 | } diff --git a/src/test/ui/async-await/issue-64130-3-other.stderr b/src/test/ui/async-await/issue-64130-3-other.stderr index 6b40cc9184df0..3475b66b375dd 100644 --- a/src/test/ui/async-await/issue-64130-3-other.stderr +++ b/src/test/ui/async-await/issue-64130-3-other.stderr @@ -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 | } diff --git a/src/test/ui/async-await/issue-64130-4-async-move.stderr b/src/test/ui/async-await/issue-64130-4-async-move.stderr index edde947764afe..fc231d394c11f 100644 --- a/src/test/ui/async-await/issue-64130-4-async-move.stderr +++ b/src/test/ui/async-await/issue-64130-4-async-move.stderr @@ -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 diff --git a/src/test/ui/async-await/issue-64130-non-send-future-diags.stderr b/src/test/ui/async-await/issue-64130-non-send-future-diags.stderr index 0f4441edb1385..f72757339cc5e 100644 --- a/src/test/ui/async-await/issue-64130-non-send-future-diags.stderr +++ b/src/test/ui/async-await/issue-64130-non-send-future-diags.stderr @@ -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 | } diff --git a/src/test/ui/async-await/issue-67252-unnamed-future.stderr b/src/test/ui/async-await/issue-67252-unnamed-future.stderr index cec40b5510148..b43478ee2070b 100644 --- a/src/test/ui/async-await/issue-67252-unnamed-future.stderr +++ b/src/test/ui/async-await/issue-67252-unnamed-future.stderr @@ -12,7 +12,7 @@ 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 | }); diff --git a/src/test/ui/async-await/issue-68112.rs b/src/test/ui/async-await/issue-68112.rs index 604cc51921e0e..11b1783680807 100644 --- a/src/test/ui/async-await/issue-68112.rs +++ b/src/test/ui/async-await/issue-68112.rs @@ -49,6 +49,8 @@ fn make_non_send_future2() -> impl Future>> { 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(); diff --git a/src/test/ui/async-await/issue-68112.stderr b/src/test/ui/async-await/issue-68112.stderr index c0659bf944b81..78462868ede53 100644 --- a/src/test/ui/async-await/issue-68112.stderr +++ b/src/test/ui/async-await/issue-68112.stderr @@ -8,13 +8,11 @@ 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` -note: future is not `Send` as this value is used in an await +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 = make_non_send_future1(); - | ------------ created here LL | let _ = non_send_fut.await; - | ^^^^^^^^^^^^ await occurs here + | ^^^^^^^^^^^^ 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 @@ -26,14 +24,14 @@ 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` -note: future is not `Send` as this value is used in an await +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 + | ^^^^^^^^^^^^^^^^^^^^^^^ await occurs here on type `impl std::future::Future`, which is not `Send` error[E0277]: `std::cell::RefCell` cannot be shared between threads safely - --> $DIR/issue-68112.rs:58:5 + --> $DIR/issue-68112.rs:60:5 | LL | fn require_send(_: impl Send) {} | ------------ ---- required by this bound in `require_send` @@ -49,8 +47,8 @@ LL | require_send(send_fut); = 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}` - = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:53:26: 57:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready}]` - = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:53:26: 57:6 {std::future::ResumeTy, impl std::future::Future, (), i32, Ready}]>` + = 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}]` + = 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}]>` = note: required because it appears within the type `impl std::future::Future` error: aborting due to 3 previous errors diff --git a/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr b/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr index a04ae7220ec88..49cd30e11a0c1 100644 --- a/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr +++ b/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr @@ -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 diff --git a/src/test/ui/generator/issue-68112.stderr b/src/test/ui/generator/issue-68112.stderr index f40771d2826d6..4148b503ba858 100644 --- a/src/test/ui/generator/issue-68112.stderr +++ b/src/test/ui/generator/issue-68112.stderr @@ -8,11 +8,11 @@ LL | require_send(send_gen); | ^^^^^^^^^^^^ generator is not `Send` | = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell` -note: generator is not `Send` as this value is used across an yield +note: generator is not `Send` as this value is used across a yield --> $DIR/issue-68112.rs:31:9 | LL | let _non_send_gen = make_non_send_generator(); - | ------------- created here + | ------------- has type `impl std::ops::Generator` which is not `Send` LL | yield; | ^^^^^ yield occurs here, with `_non_send_gen` maybe used later LL | }; diff --git a/src/test/ui/generator/not-send-sync.stderr b/src/test/ui/generator/not-send-sync.stderr index fb59ef5f433a1..5df2c1b52fb8a 100644 --- a/src/test/ui/generator/not-send-sync.stderr +++ b/src/test/ui/generator/not-send-sync.stderr @@ -21,11 +21,11 @@ LL | assert_sync(|| { | ^^^^^^^^^^^ generator is not `Sync` | = help: within `[generator@$DIR/not-send-sync.rs:9:17: 13:6 {std::cell::Cell, ()}]`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell` -note: generator is not `Sync` as this value is used across an yield +note: generator is not `Sync` as this value is used across a yield --> $DIR/not-send-sync.rs:12:9 | LL | let a = Cell::new(2); - | - has type `std::cell::Cell` + | - has type `std::cell::Cell` which is not `Sync` LL | yield; | ^^^^^ yield occurs here, with `a` maybe used later LL | }); From 4326d959f460321ed6f818a015f157af46ec5848 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Mon, 13 Apr 2020 19:14:26 -0700 Subject: [PATCH 12/12] Update test after rebase --- src/test/ui/async-await/issue-68112.stderr | 6 +++--- src/test/ui/generator/issue-68112.stderr | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/ui/async-await/issue-68112.stderr b/src/test/ui/async-await/issue-68112.stderr index 78462868ede53..6ded3e475bc37 100644 --- a/src/test/ui/async-await/issue-68112.stderr +++ b/src/test/ui/async-await/issue-68112.stderr @@ -2,7 +2,7 @@ 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` + | ---- required by this bound in `require_send` ... LL | require_send(send_fut); | ^^^^^^^^^^^^ future created by async block is not `Send` @@ -18,7 +18,7 @@ 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` + | ---- required by this bound in `require_send` ... LL | require_send(send_fut); | ^^^^^^^^^^^^ future created by async block is not `Send` @@ -34,7 +34,7 @@ error[E0277]: `std::cell::RefCell` 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` + | ---- required by this bound in `require_send` ... LL | require_send(send_fut); | ^^^^^^^^^^^^ `std::cell::RefCell` cannot be shared between threads safely diff --git a/src/test/ui/generator/issue-68112.stderr b/src/test/ui/generator/issue-68112.stderr index 4148b503ba858..83536f2af1406 100644 --- a/src/test/ui/generator/issue-68112.stderr +++ b/src/test/ui/generator/issue-68112.stderr @@ -2,7 +2,7 @@ error: generator cannot be sent between threads safely --> $DIR/issue-68112.rs:33:5 | LL | fn require_send(_: impl Send) {} - | ------------ ---- required by this bound in `require_send` + | ---- required by this bound in `require_send` ... LL | require_send(send_gen); | ^^^^^^^^^^^^ generator is not `Send` @@ -22,7 +22,7 @@ error[E0277]: `std::cell::RefCell` cannot be shared between threads safely --> $DIR/issue-68112.rs:52:5 | LL | fn require_send(_: impl Send) {} - | ------------ ---- required by this bound in `require_send` + | ---- required by this bound in `require_send` ... LL | require_send(send_gen); | ^^^^^^^^^^^^ `std::cell::RefCell` cannot be shared between threads safely