From 438455d18bb45e20cf11f06622e95ecf9c4209c4 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 6 Oct 2019 23:14:34 +0100 Subject: [PATCH 1/3] async/await: more improvements to non-send errors Signed-off-by: David Wood --- src/libcore/marker.rs | 2 + src/librustc/traits/error_reporting.rs | 294 +++++++++++++----- src/librustc_errors/diagnostic.rs | 10 + src/libstd/future.rs | 1 - src/libsyntax_pos/symbol.rs | 2 + src/test/ui/async-await/async-fn-nonsend.rs | 8 +- .../ui/async-await/async-fn-nonsend.stderr | 91 +++--- src/test/ui/async-await/issue-64130-1-sync.rs | 23 ++ .../ui/async-await/issue-64130-1-sync.stderr | 22 ++ src/test/ui/async-await/issue-64130-2-send.rs | 23 ++ .../ui/async-await/issue-64130-2-send.stderr | 22 ++ .../ui/async-await/issue-64130-3-other.rs | 25 ++ .../ui/async-await/issue-64130-3-other.stderr | 24 ++ .../async-await/issue-64130-4-async-move.rs | 28 ++ .../issue-64130-4-async-move.stderr | 22 ++ .../issue-64130-non-send-future-diags.rs | 12 +- .../issue-64130-non-send-future-diags.stderr | 11 +- src/test/ui/generator/not-send-sync.rs | 2 +- src/test/ui/generator/not-send-sync.stderr | 15 +- 19 files changed, 490 insertions(+), 147 deletions(-) create mode 100644 src/test/ui/async-await/issue-64130-1-sync.rs create mode 100644 src/test/ui/async-await/issue-64130-1-sync.stderr create mode 100644 src/test/ui/async-await/issue-64130-2-send.rs create mode 100644 src/test/ui/async-await/issue-64130-2-send.stderr create mode 100644 src/test/ui/async-await/issue-64130-3-other.rs create mode 100644 src/test/ui/async-await/issue-64130-3-other.stderr create mode 100644 src/test/ui/async-await/issue-64130-4-async-move.rs create mode 100644 src/test/ui/async-await/issue-64130-4-async-move.stderr diff --git a/src/libcore/marker.rs b/src/libcore/marker.rs index 86ee673cea941..288017b7ca53e 100644 --- a/src/libcore/marker.rs +++ b/src/libcore/marker.rs @@ -29,6 +29,7 @@ use crate::hash::Hasher; /// [arc]: ../../std/sync/struct.Arc.html /// [ub]: ../../reference/behavior-considered-undefined.html #[stable(feature = "rust1", since = "1.0.0")] +#[cfg_attr(not(test), rustc_diagnostic_item = "send_trait")] #[rustc_on_unimplemented( message="`{Self}` cannot be sent between threads safely", label="`{Self}` cannot be sent between threads safely" @@ -440,6 +441,7 @@ pub macro Copy($item:item) { /* compiler built-in */ } /// [ub]: ../../reference/behavior-considered-undefined.html /// [transmute]: ../../std/mem/fn.transmute.html #[stable(feature = "rust1", since = "1.0.0")] +#[cfg_attr(not(test), rustc_diagnostic_item = "sync_trait")] #[lang = "sync"] #[rustc_on_unimplemented( message="`{Self}` cannot be shared between threads safely", diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 35017d6330da3..53dbd186e6670 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -25,6 +25,7 @@ use crate::infer::{self, InferCtxt}; use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use crate::session::DiagnosticMessageId; use crate::ty::{self, AdtKind, DefIdTree, ToPredicate, ToPolyTraitRef, Ty, TyCtxt, TypeFoldable}; +use crate::ty::TypeckTables; use crate::ty::GenericParamDefKind; use crate::ty::error::ExpectedFound; use crate::ty::fast_reject; @@ -2104,52 +2105,69 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ) { // First, attempt to add note to this error with an async-await-specific // message, and fall back to regular note otherwise. - if !self.note_obligation_cause_for_async_await(err, obligation) { + if !self.maybe_note_obligation_cause_for_async_await(err, obligation) { self.note_obligation_cause_code(err, &obligation.predicate, &obligation.cause.code, &mut vec![]); } } - /// Adds an async-await specific note to the diagnostic: + /// Adds an async-await specific note to the diagnostic when the future does not implement + /// an auto trait because of a captured type. /// /// ```ignore (diagnostic) - /// note: future does not implement `std::marker::Send` because this value is used across an - /// await - /// --> $DIR/issue-64130-non-send-future-diags.rs:15:5 + /// note: future does not implement `Qux` as this value is used across an await + /// --> $DIR/issue-64130-3-other.rs:17:5 /// | - /// LL | let g = x.lock().unwrap(); - /// | - has type `std::sync::MutexGuard<'_, u32>` + /// LL | let x = Foo; + /// | - has type `Foo` /// LL | baz().await; - /// | ^^^^^^^^^^^ await occurs here, with `g` maybe used later + /// | ^^^^^^^^^^^ await occurs here, with `x` maybe used later /// LL | } - /// | - `g` is later dropped here + /// | - `x` is later dropped here + /// ``` + /// + /// When the diagnostic does not implement `Send` or `Sync` specifically, then the diagnostic + /// is "replaced" with a different message and a more specific error. + /// + /// ```ignore (diagnostic) + /// error: future cannot be sent between threads safely + /// --> $DIR/issue-64130-2-send.rs:21:5 + /// | + /// LL | fn is_send(t: T) { } + /// | ------- ---- required by this bound in `is_send` + /// ... + /// LL | is_send(bar()); + /// | ^^^^^^^ future returned by `bar` is not send + /// | + /// = help: within `impl std::future::Future`, the trait `std::marker::Send` is not + /// implemented for `Foo` + /// 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` + /// LL | baz().await; + /// | ^^^^^^^^^^^ await occurs here, with `x` maybe used later + /// LL | } + /// | - `x` is later dropped here /// ``` /// /// Returns `true` if an async-await specific note was added to the diagnostic. - fn note_obligation_cause_for_async_await( + fn maybe_note_obligation_cause_for_async_await( &self, err: &mut DiagnosticBuilder<'_>, obligation: &PredicateObligation<'tcx>, ) -> bool { - debug!("note_obligation_cause_for_async_await: obligation.predicate={:?} \ + debug!("maybe_note_obligation_cause_for_async_await: obligation.predicate={:?} \ obligation.cause.span={:?}", obligation.predicate, obligation.cause.span); let source_map = self.tcx.sess.source_map(); - // Look into the obligation predicate to determine the type in the generator which meant - // that the predicate was not satisifed. - let (trait_ref, target_ty) = match obligation.predicate { - ty::Predicate::Trait(trait_predicate) => - (trait_predicate.skip_binder().trait_ref, trait_predicate.skip_binder().self_ty()), - _ => return false, - }; - debug!("note_obligation_cause_for_async_await: target_ty={:?}", target_ty); - // Attempt to detect an async-await error by looking at the obligation causes, looking - // for only generators, generator witnesses, opaque types or `std::future::GenFuture` to - // be present. + // for a generator to be present. // // When a future does not implement a trait because of a captured type in one of the // generators somewhere in the call stack, then the result is a chain of obligations. + // // Given a `async fn` A that calls a `async fn` B which captures a non-send type and that // future is passed as an argument to a function C which requires a `Send` type, then the // chain looks something like this: @@ -2166,100 +2184,210 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // - `BuiltinDerivedObligation` with `impl std::future::Future` (A) // - `BindingObligation` with `impl_send (Send requirement) // - // The first obligations in the chain can be used to get the details of the type that is - // captured but the entire chain must be inspected to detect this case. + // 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. + 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())), + _ => (None, None), + }; let mut generator = None; + let mut last_generator = None; let mut next_code = Some(&obligation.cause.code); while let Some(code) = next_code { - debug!("note_obligation_cause_for_async_await: code={:?}", code); + debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code); match code { ObligationCauseCode::BuiltinDerivedObligation(derived_obligation) | ObligationCauseCode::ImplDerivedObligation(derived_obligation) => { - debug!("note_obligation_cause_for_async_await: self_ty.kind={:?}", - derived_obligation.parent_trait_ref.self_ty().kind); - match derived_obligation.parent_trait_ref.self_ty().kind { - ty::Adt(ty::AdtDef { did, .. }, ..) if - self.tcx.is_diagnostic_item(sym::gen_future, *did) => {}, - ty::Generator(did, ..) => generator = generator.or(Some(did)), - ty::GeneratorWitness(_) | ty::Opaque(..) => {}, - _ => return false, + let ty = derived_obligation.parent_trait_ref.self_ty(); + debug!("maybe_note_obligation_cause_for_async_await: \ + parent_trait_ref={:?} self_ty.kind={:?}", + derived_obligation.parent_trait_ref, ty.kind); + + match ty.kind { + ty::Generator(did, ..) => { + generator = generator.or(Some(did)); + last_generator = Some(did); + }, + ty::GeneratorWitness(..) => {}, + _ if generator.is_none() => { + trait_ref = Some(*derived_obligation.parent_trait_ref.skip_binder()); + target_ty = Some(ty); + }, + _ => {}, } next_code = Some(derived_obligation.parent_code.as_ref()); }, - ObligationCauseCode::ItemObligation(_) | ObligationCauseCode::BindingObligation(..) - if generator.is_some() => break, - _ => return false, + _ => break, } } - let generator_did = generator.expect("can only reach this if there was a generator"); - - // Only continue to add a note if the generator is from an `async` function. - let parent_node = self.tcx.parent(generator_did) - .and_then(|parent_did| self.tcx.hir().get_if_local(parent_did)); - debug!("note_obligation_cause_for_async_await: parent_node={:?}", parent_node); - if let Some(hir::Node::Item(hir::Item { - kind: hir::ItemKind::Fn(sig, _, _), - .. - })) = parent_node { - debug!("note_obligation_cause_for_async_await: header={:?}", sig.header); - if sig.header.asyncness != hir::IsAsync::Async { - return false; - } - } + // Only continue if a generator was found. + debug!("maybe_note_obligation_cause_for_async_await: generator={:?} trait_ref={:?} \ + target_ty={:?}", generator, trait_ref, target_ty); + let (generator_did, trait_ref, target_ty) = match (generator, trait_ref, target_ty) { + (Some(generator_did), Some(trait_ref), Some(target_ty)) => + (generator_did, trait_ref, target_ty), + _ => return false, + }; let span = self.tcx.def_span(generator_did); + // Do not ICE on closure typeck (#66868). if let None = self.tcx.hir().as_local_hir_id(generator_did) { return false; } - let tables = self.tcx.typeck_tables_of(generator_did); - debug!("note_obligation_cause_for_async_await: generator_did={:?} span={:?} ", - generator_did, span); + + // Get the tables from the infcx if the generator is the function we are + // currently type-checking; otherwise, get them by performing a query. + // This is needed to avoid cycles. + let in_progress_tables = self.in_progress_tables.map(|t| t.borrow()); + let generator_did_root = self.tcx.closure_base_def_id(generator_did); + debug!("maybe_note_obligation_cause_for_async_await: generator_did={:?} \ + generator_did_root={:?} in_progress_tables.local_id_root={:?} span={:?}", + generator_did, generator_did_root, + in_progress_tables.as_ref().map(|t| t.local_id_root), span); + let query_tables; + let tables: &TypeckTables<'tcx> = match &in_progress_tables { + Some(t) if t.local_id_root == Some(generator_did_root) => t, + _ => { + query_tables = self.tcx.typeck_tables_of(generator_did); + &query_tables + } + }; // Look for a type inside the generator interior that matches the target type to get // a span. let target_span = tables.generator_interior_types.iter() - .find(|ty::GeneratorInteriorTypeCause { ty, .. }| ty::TyS::same_type(*ty, target_ty)) + .find(|ty::GeneratorInteriorTypeCause { ty, .. }| { + let ty = ty.builtin_deref(false).map(|ty_and_mut| ty_and_mut.ty).unwrap_or(ty); + let target_ty = target_ty.builtin_deref(false) + .map(|ty_and_mut| ty_and_mut.ty) + .unwrap_or(target_ty); + let eq = ty::TyS::same_type(ty, target_ty); + debug!("maybe_note_obligation_cause_for_async_await: ty={:?} \ + target_ty={:?} eq={:?}", ty, target_ty, eq); + eq + }) .map(|ty::GeneratorInteriorTypeCause { span, scope_span, .. }| (span, source_map.span_to_snippet(*span), scope_span)); + debug!("maybe_note_obligation_cause_for_async_await: target_ty={:?} \ + generator_interior_types={:?} target_span={:?}", + target_ty, tables.generator_interior_types, target_span); if let Some((target_span, Ok(snippet), scope_span)) = target_span { - // Look at the last interior type to get a span for the `.await`. - let await_span = tables.generator_interior_types.iter().map(|i| i.span).last().unwrap(); - let mut span = MultiSpan::from_span(await_span); - span.push_span_label( - await_span, format!("await occurs here, with `{}` maybe used later", snippet)); - - span.push_span_label(*target_span, format!("has type `{}`", target_ty)); + self.note_obligation_cause_for_async_await( + err, *target_span, scope_span, snippet, generator_did, last_generator, + trait_ref, target_ty, tables, obligation, next_code, + ); + true + } else { + false + } + } - // 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), - ); - } + /// Unconditionally adds the diagnostic note described in + /// `maybe_note_obligation_cause_for_async_await`'s documentation comment. + fn note_obligation_cause_for_async_await( + &self, + err: &mut DiagnosticBuilder<'_>, + target_span: Span, + scope_span: &Option, + snippet: String, + first_generator: DefId, + last_generator: Option, + trait_ref: ty::TraitRef<'_>, + target_ty: Ty<'tcx>, + tables: &ty::TypeckTables<'_>, + obligation: &PredicateObligation<'tcx>, + next_code: Option<&ObligationCauseCode<'tcx>>, + ) { + let source_map = self.tcx.sess.source_map(); - err.span_note(span, &format!( - "future does not implement `{}` as this value is used across an await", - trait_ref.print_only_trait_path(), - )); + let is_async_fn = self.tcx.parent(first_generator) + .and_then(|parent_did| self.tcx.hir().get_if_local(parent_did)) + .and_then(|parent_node| match parent_node { + Node::Item(item) => Some(&item.kind), + _ => None, + }) + .and_then(|parent_item_kind| match parent_item_kind { + hir::ItemKind::Fn(_, hir::FnHeader { asyncness, .. }, _, _) => Some(asyncness), + _ => None, + }) + .map(|parent_asyncness| *parent_asyncness == hir::IsAsync::Async) + .unwrap_or(false); + let await_or_yield = if is_async_fn { "await" } else { "yield" }; + + // Special case the primary error message when send or sync is the trait that was + // not implemented. + let is_send = self.tcx.is_diagnostic_item(sym::send_trait, trait_ref.def_id); + let is_sync = self.tcx.is_diagnostic_item(sym::sync_trait, trait_ref.def_id); + let trait_explanation = if is_send || is_sync { + let (trait_name, trait_verb) = if is_send { + ("`Send`", "sent") + } else { + ("`Sync`", "shared") + }; - // Add a note for the item obligation that remains - normally a note pointing to the - // bound that introduced the obligation (e.g. `T: Send`). - debug!("note_obligation_cause_for_async_await: next_code={:?}", next_code); - self.note_obligation_cause_code( - err, - &obligation.predicate, - next_code.unwrap(), - &mut Vec::new(), + err.clear_code(); + err.set_primary_message( + format!("future cannot be {} between threads safely", trait_verb) ); - true + let original_span = err.span.primary_span().unwrap(); + let mut span = MultiSpan::from_span(original_span); + + let message = if let Some(name) = last_generator + .and_then(|generator_did| self.tcx.parent(generator_did)) + .and_then(|parent_did| self.tcx.hir().as_local_hir_id(parent_did)) + .map(|parent_hir_id| self.tcx.hir().name(parent_hir_id)) + { + format!("future returned by `{}` is not {}", name, trait_name) + } else { + format!("future is not {}", trait_name) + }; + + span.push_span_label(original_span, message); + err.set_span(span); + + format!("is not {}", trait_name) } else { - false + format!("does not implement `{}`", trait_ref.print_only_trait_path()) + }; + + // Look at the last interior type to get a span for the `.await`. + let await_span = tables.generator_interior_types.iter().map(|i| i.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)); + + 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 { + span.push_span_label( + source_map.end_point(*scope_span), + format!("`{}` is later dropped here", snippet), + ); } + + err.span_note(span, &format!( + "future {} as this value is used across an {}", + trait_explanation, + await_or_yield, + )); + + // Add a note for the item obligation that remains - normally a note pointing to the + // bound that introduced the obligation (e.g. `T: Send`). + debug!("note_obligation_cause_for_async_await: next_code={:?}", next_code); + self.note_obligation_cause_code( + err, + &obligation.predicate, + next_code.unwrap(), + &mut Vec::new(), + ); } fn note_obligation_cause_code(&self, diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index abec979054e16..744f4a47b6035 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -498,10 +498,20 @@ impl Diagnostic { self } + pub fn clear_code(&mut self) -> &mut Self { + self.code = None; + self + } + pub fn get_code(&self) -> Option { self.code.clone() } + pub fn set_primary_message>(&mut self, msg: M) -> &mut Self { + self.message[0] = (msg.into(), Style::NoStyle); + self + } + pub fn message(&self) -> String { self.message.iter().map(|i| i.0.as_str()).collect::() } diff --git a/src/libstd/future.rs b/src/libstd/future.rs index 6de3f1d545b57..ac1ef3e1d8b72 100644 --- a/src/libstd/future.rs +++ b/src/libstd/future.rs @@ -26,7 +26,6 @@ pub fn from_generator>(x: T) -> impl Future>(T); // We rely on the fact that async/await futures are immovable in order to create diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index e8f7a125739ac..92de56bd09a7a 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -660,6 +660,7 @@ symbols! { _Self, self_in_typedefs, self_struct_ctor, + send_trait, should_panic, simd, simd_extract, @@ -697,6 +698,7 @@ symbols! { sty, sub_with_overflow, suggestion, + sync_trait, target_feature, target_has_atomic, target_has_atomic_load_store, diff --git a/src/test/ui/async-await/async-fn-nonsend.rs b/src/test/ui/async-await/async-fn-nonsend.rs index 1f1bf4250eadf..645c903c6bab2 100644 --- a/src/test/ui/async-await/async-fn-nonsend.rs +++ b/src/test/ui/async-await/async-fn-nonsend.rs @@ -48,10 +48,10 @@ fn assert_send(_: impl Send) {} pub fn pass_assert() { assert_send(local_dropped_before_await()); - //~^ ERROR `std::rc::Rc<()>` cannot be sent between threads safely + //~^ ERROR future cannot be sent between threads safely assert_send(non_send_temporary_in_match()); - //~^ ERROR `std::rc::Rc<()>` cannot be sent between threads safely + //~^ ERROR future cannot be sent between threads safely assert_send(non_sync_with_method_call()); - //~^ ERROR `dyn std::fmt::Write` cannot be sent between threads safely - //~^^ ERROR `*mut (dyn std::ops::Fn() + 'static)` cannot be shared between threads safely + //~^ ERROR future cannot be sent between threads safely + //~^^ ERROR future cannot be sent between threads safely } diff --git a/src/test/ui/async-await/async-fn-nonsend.stderr b/src/test/ui/async-await/async-fn-nonsend.stderr index 6e89deb407e92..5c870ca2d0276 100644 --- a/src/test/ui/async-await/async-fn-nonsend.stderr +++ b/src/test/ui/async-await/async-fn-nonsend.stderr @@ -1,79 +1,88 @@ -error[E0277]: `std::rc::Rc<()>` cannot be sent between threads safely +error: future cannot be sent between threads safely --> $DIR/async-fn-nonsend.rs:50:5 | LL | fn assert_send(_: impl Send) {} | ----------- ---- required by this bound in `assert_send` ... LL | assert_send(local_dropped_before_await()); - | ^^^^^^^^^^^ `std::rc::Rc<()>` cannot be sent between threads safely + | ^^^^^^^^^^^ future returned by `local_dropped_before_await` is not `Send` | = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<()>` - = note: required because it appears within the type `impl std::fmt::Debug` - = note: required because it appears within the type `{impl std::fmt::Debug, impl std::future::Future, impl std::future::Future, ()}` - = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:21:39: 26:2 {impl std::fmt::Debug, impl std::future::Future, impl std::future::Future, ()}]` - = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:21:39: 26:2 {impl std::fmt::Debug, impl std::future::Future, 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: future is not `Send` as this value is used across an await + --> $DIR/async-fn-nonsend.rs:25:5 + | +LL | let x = non_send(); + | - has type `impl std::fmt::Debug` +LL | drop(x); +LL | fut().await; + | ^^^^^^^^^^^ await occurs here, with `x` maybe used later +LL | } + | - `x` is later dropped here -error[E0277]: `std::rc::Rc<()>` cannot be sent between threads safely +error: future cannot be sent between threads safely --> $DIR/async-fn-nonsend.rs:52:5 | LL | fn assert_send(_: impl Send) {} | ----------- ---- required by this bound in `assert_send` ... LL | assert_send(non_send_temporary_in_match()); - | ^^^^^^^^^^^ `std::rc::Rc<()>` cannot be sent between threads safely + | ^^^^^^^^^^^ future returned by `non_send_temporary_in_match` is not `Send` | = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<()>` - = note: required because it appears within the type `impl std::fmt::Debug` - = note: required because it appears within the type `{impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}` - = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:28:40: 37:2 {impl std::fmt::Debug, std::option::Option, impl std::future::Future, impl std::future::Future, ()}]` - = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:28:40: 37:2 {impl std::fmt::Debug, std::option::Option, impl std::future::Future, 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: future is not `Send` as this value is used across an await + --> $DIR/async-fn-nonsend.rs:34:20 + | +LL | match Some(non_send()) { + | ---------- has type `impl std::fmt::Debug` +LL | Some(_) => fut().await, + | ^^^^^^^^^^^ await occurs here, with `non_send()` maybe used later +... +LL | } + | - `non_send()` is later dropped here -error[E0277]: `dyn std::fmt::Write` cannot be sent between threads safely +error: future cannot be sent between threads safely --> $DIR/async-fn-nonsend.rs:54:5 | LL | fn assert_send(_: impl Send) {} | ----------- ---- required by this bound in `assert_send` ... LL | assert_send(non_sync_with_method_call()); - | ^^^^^^^^^^^ `dyn std::fmt::Write` cannot be sent between threads safely + | ^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send` | = help: the trait `std::marker::Send` is not implemented for `dyn std::fmt::Write` - = note: required because of the requirements on the impl of `std::marker::Send` for `&mut dyn std::fmt::Write` - = note: required because it appears within the type `std::fmt::Formatter<'_>` - = note: required because of the requirements on the impl of `std::marker::Send` for `&mut std::fmt::Formatter<'_>` - = note: required because it appears within the type `for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, impl std::future::Future, ()}` - = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:39:38: 45:2 for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, impl std::future::Future, ()}]` - = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:39:38: 45:2 for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, 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: future is not `Send` as this value is used across an await + --> $DIR/async-fn-nonsend.rs:43:9 + | +LL | let f: &mut std::fmt::Formatter = panic!(); + | - has type `&mut std::fmt::Formatter<'_>` +LL | if non_sync().fmt(f).unwrap() == () { +LL | fut().await; + | ^^^^^^^^^^^ await occurs here, with `f` maybe used later +LL | } +LL | } + | - `f` is later dropped here -error[E0277]: `*mut (dyn std::ops::Fn() + 'static)` cannot be shared between threads safely +error: future cannot be sent between threads safely --> $DIR/async-fn-nonsend.rs:54:5 | LL | fn assert_send(_: impl Send) {} | ----------- ---- required by this bound in `assert_send` ... LL | assert_send(non_sync_with_method_call()); - | ^^^^^^^^^^^ `*mut (dyn std::ops::Fn() + 'static)` cannot be shared between threads safely + | ^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send` | = help: within `std::fmt::ArgumentV1<'_>`, the trait `std::marker::Sync` is not implemented for `*mut (dyn std::ops::Fn() + 'static)` - = note: required because it appears within the type `std::marker::PhantomData<*mut (dyn std::ops::Fn() + 'static)>` - = note: required because it appears within the type `core::fmt::Void` - = note: required because it appears within the type `&core::fmt::Void` - = note: required because it appears within the type `std::fmt::ArgumentV1<'_>` - = note: required because of the requirements on the impl of `std::marker::Send` for `std::slice::Iter<'_, std::fmt::ArgumentV1<'_>>` - = note: required because it appears within the type `std::fmt::Formatter<'_>` - = note: required because of the requirements on the impl of `std::marker::Send` for `&mut std::fmt::Formatter<'_>` - = note: required because it appears within the type `for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, impl std::future::Future, ()}` - = note: required because it appears within the type `[static generator@$DIR/async-fn-nonsend.rs:39:38: 45:2 for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, impl std::future::Future, ()}]` - = note: required because it appears within the type `std::future::GenFuture<[static generator@$DIR/async-fn-nonsend.rs:39:38: 45:2 for<'r, 's> {&'r mut std::fmt::Formatter<'s>, bool, bool, impl std::future::Future, 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: future is not `Send` as this value is used across an await + --> $DIR/async-fn-nonsend.rs:43:9 + | +LL | let f: &mut std::fmt::Formatter = panic!(); + | - has type `&mut std::fmt::Formatter<'_>` +LL | if non_sync().fmt(f).unwrap() == () { +LL | fut().await; + | ^^^^^^^^^^^ await occurs here, with `f` maybe used later +LL | } +LL | } + | - `f` is later dropped here error: aborting due to 4 previous errors -For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/async-await/issue-64130-1-sync.rs b/src/test/ui/async-await/issue-64130-1-sync.rs new file mode 100644 index 0000000000000..cc5ca89f03af0 --- /dev/null +++ b/src/test/ui/async-await/issue-64130-1-sync.rs @@ -0,0 +1,23 @@ +#![feature(optin_builtin_traits)] +// edition:2018 + +// This tests the the specialized async-await-specific error when futures don't implement an +// auto trait (which is specifically Sync) due to some type that was captured. + +struct Foo; + +impl !Sync for Foo {} + +fn is_sync(t: T) { } + +async fn bar() { + let x = Foo; + baz().await; +} + +async fn baz() { } + +fn main() { + is_sync(bar()); + //~^ ERROR future cannot be shared between threads safely +} diff --git a/src/test/ui/async-await/issue-64130-1-sync.stderr b/src/test/ui/async-await/issue-64130-1-sync.stderr new file mode 100644 index 0000000000000..8beb31f152a9d --- /dev/null +++ b/src/test/ui/async-await/issue-64130-1-sync.stderr @@ -0,0 +1,22 @@ +error: future cannot be shared between threads safely + --> $DIR/issue-64130-1-sync.rs:21:5 + | +LL | fn is_sync(t: T) { } + | ------- ---- required by this bound in `is_sync` +... +LL | is_sync(bar()); + | ^^^^^^^ future returned by `bar` is not `Sync` + | + = help: within `impl std::future::Future`, the trait `std::marker::Sync` is not implemented for `Foo` +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` +LL | baz().await; + | ^^^^^^^^^^^ await occurs here, with `x` maybe used later +LL | } + | - `x` is later dropped here + +error: aborting due to previous error + diff --git a/src/test/ui/async-await/issue-64130-2-send.rs b/src/test/ui/async-await/issue-64130-2-send.rs new file mode 100644 index 0000000000000..1efe2ab3f85e2 --- /dev/null +++ b/src/test/ui/async-await/issue-64130-2-send.rs @@ -0,0 +1,23 @@ +#![feature(optin_builtin_traits)] +// edition:2018 + +// This tests the the specialized async-await-specific error when futures don't implement an +// auto trait (which is specifically Send) due to some type that was captured. + +struct Foo; + +impl !Send for Foo {} + +fn is_send(t: T) { } + +async fn bar() { + let x = Foo; + baz().await; +} + +async fn baz() { } + +fn main() { + is_send(bar()); + //~^ ERROR future cannot be sent between threads safely +} diff --git a/src/test/ui/async-await/issue-64130-2-send.stderr b/src/test/ui/async-await/issue-64130-2-send.stderr new file mode 100644 index 0000000000000..823b88e18c5b6 --- /dev/null +++ b/src/test/ui/async-await/issue-64130-2-send.stderr @@ -0,0 +1,22 @@ +error: future cannot be sent between threads safely + --> $DIR/issue-64130-2-send.rs:21:5 + | +LL | fn is_send(t: T) { } + | ------- ---- required by this bound in `is_send` +... +LL | is_send(bar()); + | ^^^^^^^ future returned by `bar` is not `Send` + | + = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `Foo` +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` +LL | baz().await; + | ^^^^^^^^^^^ await occurs here, with `x` maybe used later +LL | } + | - `x` is later dropped here + +error: aborting due to previous error + diff --git a/src/test/ui/async-await/issue-64130-3-other.rs b/src/test/ui/async-await/issue-64130-3-other.rs new file mode 100644 index 0000000000000..901544edba18a --- /dev/null +++ b/src/test/ui/async-await/issue-64130-3-other.rs @@ -0,0 +1,25 @@ +#![feature(optin_builtin_traits)] +// edition:2018 + +// This tests the the unspecialized async-await-specific error when futures don't implement an +// auto trait (which is not Send or Sync) due to some type that was captured. + +auto trait Qux { } + +struct Foo; + +impl !Qux for Foo {} + +fn is_qux(t: T) { } + +async fn bar() { + let x = Foo; + baz().await; +} + +async fn baz() { } + +fn main() { + is_qux(bar()); + //~^ ERROR the trait bound `Foo: Qux` is not satisfied in `impl std::future::Future` +} diff --git a/src/test/ui/async-await/issue-64130-3-other.stderr b/src/test/ui/async-await/issue-64130-3-other.stderr new file mode 100644 index 0000000000000..155c5cc8ea137 --- /dev/null +++ b/src/test/ui/async-await/issue-64130-3-other.stderr @@ -0,0 +1,24 @@ +error[E0277]: the trait bound `Foo: Qux` is not satisfied in `impl std::future::Future` + --> $DIR/issue-64130-3-other.rs:23:5 + | +LL | fn is_qux(t: T) { } + | ------ --- required by this bound in `is_qux` +... +LL | is_qux(bar()); + | ^^^^^^ within `impl std::future::Future`, the trait `Qux` is not implemented for `Foo` + | + = help: the following implementations were found: + +note: future does not implement `Qux` as this value is used across an await + --> $DIR/issue-64130-3-other.rs:17:5 + | +LL | let x = Foo; + | - has type `Foo` +LL | baz().await; + | ^^^^^^^^^^^ await occurs here, with `x` maybe used later +LL | } + | - `x` is later dropped here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/async-await/issue-64130-4-async-move.rs b/src/test/ui/async-await/issue-64130-4-async-move.rs new file mode 100644 index 0000000000000..2538f34351e5a --- /dev/null +++ b/src/test/ui/async-await/issue-64130-4-async-move.rs @@ -0,0 +1,28 @@ +// edition:2018 +use std::any::Any; +use std::future::Future; + +struct Client(Box); + +impl Client { + fn status(&self) -> u16 { + 200 + } +} + +async fn get() { } + +pub fn foo() -> impl Future + Send { + //~^ ERROR future cannot be sent between threads safely + let client = Client(Box::new(true)); + async move { + match client.status() { + 200 => { + let _x = get().await; + }, + _ => (), + } + } +} + +fn main() {} 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 new file mode 100644 index 0000000000000..3def984972f84 --- /dev/null +++ b/src/test/ui/async-await/issue-64130-4-async-move.stderr @@ -0,0 +1,22 @@ +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` + | + = help: the trait `std::marker::Sync` is not implemented for `(dyn std::any::Any + std::marker::Send + 'static)` +note: future is not `Send` as this value is used across an yield + --> $DIR/issue-64130-4-async-move.rs:21:26 + | +LL | match client.status() { + | ------ has type `&Client` +LL | 200 => { +LL | let _x = get().await; + | ^^^^^^^^^^^ yield occurs here, with `client` maybe used later +... +LL | } + | - `client` is later dropped here + = note: the return type of a function must have a statically known size + +error: aborting due to previous error + diff --git a/src/test/ui/async-await/issue-64130-non-send-future-diags.rs b/src/test/ui/async-await/issue-64130-non-send-future-diags.rs index 1936d1a2ed56e..656ade67c71a7 100644 --- a/src/test/ui/async-await/issue-64130-non-send-future-diags.rs +++ b/src/test/ui/async-await/issue-64130-non-send-future-diags.rs @@ -1,10 +1,10 @@ // edition:2018 -use std::sync::Mutex; +// This tests the basic example case for the async-await-specific error. -fn is_send(t: T) { +use std::sync::Mutex; -} +fn is_send(t: T) { } async fn foo() { bar(&Mutex::new(22)).await; @@ -15,11 +15,9 @@ async fn bar(x: &Mutex) { baz().await; } -async fn baz() { - -} +async fn baz() { } fn main() { is_send(foo()); - //~^ ERROR `std::sync::MutexGuard<'_, u32>` cannot be sent between threads safely [E0277] + //~^ ERROR future cannot be sent between threads safely } 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 9e9fc52e30b7f..662407f7017f5 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 @@ -1,14 +1,14 @@ -error[E0277]: `std::sync::MutexGuard<'_, u32>` cannot be sent between threads safely - --> $DIR/issue-64130-non-send-future-diags.rs:23:5 +error: future cannot be sent between threads safely + --> $DIR/issue-64130-non-send-future-diags.rs:21:5 | -LL | fn is_send(t: T) { +LL | fn is_send(t: T) { } | ------- ---- required by this bound in `is_send` ... LL | is_send(foo()); - | ^^^^^^^ `std::sync::MutexGuard<'_, u32>` cannot be sent between threads safely + | ^^^^^^^ future returned by `foo` is not `Send` | = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `std::sync::MutexGuard<'_, u32>` -note: future does not implement `std::marker::Send` as this value is used across an await +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(); @@ -20,4 +20,3 @@ LL | } error: aborting due to previous error -For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/generator/not-send-sync.rs b/src/test/ui/generator/not-send-sync.rs index ae0a288bbb496..0db01c6f756ac 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: E0277 + //~^ ERROR: future 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 620db245d3e57..0ac1d189b79b0 100644 --- a/src/test/ui/generator/not-send-sync.stderr +++ b/src/test/ui/generator/not-send-sync.stderr @@ -11,18 +11,25 @@ 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[E0277]: `std::cell::Cell` cannot be shared between threads safely +error: future cannot be shared between threads safely --> $DIR/not-send-sync.rs:9:5 | LL | fn assert_sync(_: T) {} | ----------- ---- required by this bound in `main::assert_sync` ... LL | assert_sync(|| { - | ^^^^^^^^^^^ `std::cell::Cell` cannot be shared between threads safely + | ^^^^^^^^^^^ future returned by `main` 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: required because it appears within the type `{std::cell::Cell, ()}` - = note: required because it appears within the type `[generator@$DIR/not-send-sync.rs:9:17: 13:6 {std::cell::Cell, ()}]` +note: future is not `Sync` as this value is used across an yield + --> $DIR/not-send-sync.rs:12:9 + | +LL | let a = Cell::new(2); + | - has type `std::cell::Cell` +LL | yield; + | ^^^^^ yield occurs here, with `a` maybe used later +LL | }); + | - `a` is later dropped here error: aborting due to 2 previous errors From f0b51145c5006f79da8304e1fb09d6f0eb95ae1a Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 31 Oct 2019 14:36:41 +0000 Subject: [PATCH 2/3] async/await: correct diag note for `async move` This commit corrects the diagnostic note for `async move {}` so that `await` is mentioned, rather than `yield`. Signed-off-by: David Wood --- src/librustc/traits/error_reporting.rs | 21 ++++++++++--------- .../issue-64130-4-async-move.stderr | 4 ++-- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 53dbd186e6670..84ffb74045eff 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -2306,18 +2306,19 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let source_map = self.tcx.sess.source_map(); let is_async_fn = self.tcx.parent(first_generator) - .and_then(|parent_did| self.tcx.hir().get_if_local(parent_did)) - .and_then(|parent_node| match parent_node { - Node::Item(item) => Some(&item.kind), - _ => None, - }) - .and_then(|parent_item_kind| match parent_item_kind { - hir::ItemKind::Fn(_, hir::FnHeader { asyncness, .. }, _, _) => Some(asyncness), - _ => None, + .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) + .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()) + .map(|generator_kind| match generator_kind { + hir::GeneratorKind::Async(..) => true, + _ => false, }) - .map(|parent_asyncness| *parent_asyncness == hir::IsAsync::Async) .unwrap_or(false); - let await_or_yield = if is_async_fn { "await" } else { "yield" }; + let await_or_yield = if is_async_fn || is_async_move { "await" } else { "yield" }; // Special case the primary error message when send or sync is the trait that was // not implemented. 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 3def984972f84..ddbb469b99c24 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 @@ -5,14 +5,14 @@ LL | pub fn foo() -> impl Future + Send { | ^^^^^^^^^^^^^^^^^^ future returned by `foo` is not `Send` | = help: the trait `std::marker::Sync` is not implemented for `(dyn std::any::Any + std::marker::Send + 'static)` -note: future is not `Send` as this value is used across an yield +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` LL | 200 => { LL | let _x = get().await; - | ^^^^^^^^^^^ yield occurs here, with `client` maybe used later + | ^^^^^^^^^^^ await occurs here, with `client` maybe used later ... LL | } | - `client` is later dropped here From 5cd9f22464a3ae2620c384094986d9549eca182e Mon Sep 17 00:00:00 2001 From: Nicholas Matsakis Date: Wed, 11 Dec 2019 13:23:07 -0500 Subject: [PATCH 3/3] erase regions instead of using `builtin_deref` The reason we were invoking `builtin_deref` was to enable comparisons when the type was `&T`. For the reasons outlined in the comment, those comparisons failed because the regions disagreed. --- src/librustc/traits/error_reporting.rs | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 84ffb74045eff..6a111895b5637 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -2260,15 +2260,26 @@ impl<'a, 'tcx> 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 target_span = tables.generator_interior_types.iter() .find(|ty::GeneratorInteriorTypeCause { ty, .. }| { - let ty = ty.builtin_deref(false).map(|ty_and_mut| ty_and_mut.ty).unwrap_or(ty); - let target_ty = target_ty.builtin_deref(false) - .map(|ty_and_mut| ty_and_mut.ty) - .unwrap_or(target_ty); - let eq = ty::TyS::same_type(ty, target_ty); - debug!("maybe_note_obligation_cause_for_async_await: ty={:?} \ - target_ty={:?} eq={:?}", ty, target_ty, eq); + // 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 }) .map(|ty::GeneratorInteriorTypeCause { span, scope_span, .. }|