async/await: improve obligation errors

This commit improves obligation errors for async/await:

```
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
   |
LL |     let g = x.lock().unwrap();
   |         - has type `std::sync::MutexGuard<'_, u32>`
LL |     baz().await;
   |     ^^^^^^^^^^^ await occurs here, with `g` maybe used later
LL | }
   | - `g` is later dropped here
```

Signed-off-by: David Wood <[email protected]>

Author: davidtwco

src/librustc/traits/error_reporting.rs

@@ -24,7 +24,7 @@ use crate::hir::def_id::DefId;
 use crate::infer::{self, InferCtxt};
 use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use crate::session::DiagnosticMessageId;
-use crate::ty::{self, AdtKind, ToPredicate, ToPolyTraitRef, Ty, TyCtxt, TypeFoldable};
+use crate::ty::{self, AdtKind, DefIdTree, ToPredicate, ToPolyTraitRef, Ty, TyCtxt, TypeFoldable};
 use crate::ty::GenericParamDefKind;
 use crate::ty::error::ExpectedFound;
 use crate::ty::fast_reject;
@@ -37,7 +37,7 @@ use errors::{Applicability, DiagnosticBuilder, pluralise};
 use std::fmt;
 use syntax::ast;
 use syntax::symbol::{sym, kw};
-use syntax_pos::{DUMMY_SP, Span, ExpnKind};
+use syntax_pos::{DUMMY_SP, Span, ExpnKind, MultiSpan};
 
 impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     pub fn report_fulfillment_errors(
@@ -550,7 +550,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             self.suggest_new_overflow_limit(&mut err);
         }
 
-        self.note_obligation_cause(&mut err, obligation);
+        self.note_obligation_cause_code(&mut err, &obligation.predicate, &obligation.cause.code,
+                                        &mut vec![]);
 
         err.emit();
         self.tcx.sess.abort_if_errors();
@@ -940,7 +941,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                 bug!("overflow should be handled before the `report_selection_error` path");
             }
         };
+
         self.note_obligation_cause(&mut err, obligation);
+
         err.emit();
     }
 
@@ -1604,15 +1607,165 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         })
     }
 
-    fn note_obligation_cause<T>(&self,
-                                err: &mut DiagnosticBuilder<'_>,
-                                obligation: &Obligation<'tcx, T>)
-        where T: fmt::Display
-    {
-        self.note_obligation_cause_code(err,
-                                        &obligation.predicate,
-                                        &obligation.cause.code,
-                                        &mut vec![]);
+    fn note_obligation_cause(
+        &self,
+        err: &mut DiagnosticBuilder<'_>,
+        obligation: &PredicateObligation<'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) {
+            self.note_obligation_cause_code(err, &obligation.predicate, &obligation.cause.code,
+                                            &mut vec![]);
+        }
+    }
+
+    /// Adds an async-await specific note to the diagnostic:
+    ///
+    /// ```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
+    ///    |
+    /// LL |     let g = x.lock().unwrap();
+    ///    |         - has type `std::sync::MutexGuard<'_, u32>`
+    /// LL |     baz().await;
+    ///    |     ^^^^^^^^^^^ await occurs here, with `g` maybe used later
+    /// LL | }
+    ///    | - `g` is later dropped here
+    /// ```
+    ///
+    /// Returns `true` if an async-await specific note was added to the diagnostic.
+    fn note_obligation_cause_for_async_await(
+        &self,
+        err: &mut DiagnosticBuilder<'_>,
+        obligation: &PredicateObligation<'tcx>,
+    ) -> bool {
+        debug!("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.
+        //
+        // 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:
+        //
+        // - `BuiltinDerivedObligation` with a generator witness (B)
+        // - `BuiltinDerivedObligation` with a generator (B)
+        // - `BuiltinDerivedObligation` with `std::future::GenFuture` (B)
+        // - `BuiltinDerivedObligation` with `impl std::future::Future` (B)
+        // - `BuiltinDerivedObligation` with `impl std::future::Future` (B)
+        // - `BuiltinDerivedObligation` with a generator witness (A)
+        // - `BuiltinDerivedObligation` with a generator (A)
+        // - `BuiltinDerivedObligation` with `std::future::GenFuture` (A)
+        // - `BuiltinDerivedObligation` with `impl std::future::Future` (A)
+        // - `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.
+        let mut 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);
+            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,
+                    }
+
+                    next_code = Some(derived_obligation.parent_code.as_ref());
+                },
+                ObligationCauseCode::ItemObligation(_) | ObligationCauseCode::BindingObligation(..)
+                    if generator.is_some() => break,
+                _ => return false,
+            }
+        }
+
+        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(_, header, _, _),
+            ..
+        })) = parent_node {
+            debug!("note_obligation_cause_for_async_await: header={:?}", header);
+            if header.asyncness != hir::IsAsync::Async {
+                return false;
+            }
+        }
+
+        let span = self.tcx.def_span(generator_did);
+        let tables = self.tcx.typeck_tables_of(generator_did);
+        debug!("note_obligation_cause_for_async_await: generator_did={:?} span={:?} ",
+               generator_did, span);
+
+        // 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))
+            .map(|ty::GeneratorInteriorTypeCause { span, scope_span, .. }|
+                 (span, source_map.span_to_snippet(*span), scope_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));
+
+            // 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 does not implement `{}` as this value is used across an await",
+                trait_ref,
+            ));
+
+            // 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(),
+            );
+
+            true
+        } else {
+            false
+        }
     }
 
     fn note_obligation_cause_code<T>(&self,

src/librustc/ty/context.rs

@@ -288,6 +288,34 @@ pub struct ResolvedOpaqueTy<'tcx> {
     pub substs: SubstsRef<'tcx>,
 }
 
+/// Whenever a value may be live across a generator yield, the type of that value winds up in the
+/// `GeneratorInteriorTypeCause` struct. This struct adds additional information about such
+/// captured types that can be useful for diagnostics. In particular, it stores the span that
+/// caused a given type to be recorded, along with the scope that enclosed the value (which can
+/// be used to find the await that the value is live across).
+///
+/// For example:
+///
+/// ```ignore (pseudo-Rust)
+/// async move {
+///     let x: T = ...;
+///     foo.await
+///     ...
+/// }
+/// ```
+///
+/// Here, we would store the type `T`, the span of the value `x`, and the "scope-span" for
+/// the scope that contains `x`.
+#[derive(RustcEncodable, RustcDecodable, Clone, Debug, Eq, Hash, HashStable, PartialEq)]
+pub struct GeneratorInteriorTypeCause<'tcx> {
+    /// Type of the captured binding.
+    pub ty: Ty<'tcx>,
+    /// Span of the binding that was captured.
+    pub span: Span,
+    /// Span of the scope of the captured binding.
+    pub scope_span: Option<Span>,
+}
+
 #[derive(RustcEncodable, RustcDecodable, Debug)]
 pub struct TypeckTables<'tcx> {
     /// The HirId::owner all ItemLocalIds in this table are relative to.
@@ -397,6 +425,10 @@ pub struct TypeckTables<'tcx> {
     /// leading to the member of the struct or tuple that is used instead of the
     /// entire variable.
     pub upvar_list: ty::UpvarListMap,
+
+    /// Stores the type, span and optional scope span of all types
+    /// that are live across the yield of this generator (if a generator).
+    pub generator_interior_types: Vec<GeneratorInteriorTypeCause<'tcx>>,
 }
 
 impl<'tcx> TypeckTables<'tcx> {
@@ -422,6 +454,7 @@ impl<'tcx> TypeckTables<'tcx> {
             free_region_map: Default::default(),
             concrete_opaque_types: Default::default(),
             upvar_list: Default::default(),
+            generator_interior_types: Default::default(),
         }
     }
 
@@ -729,6 +762,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckTables<'tcx> {
             ref free_region_map,
             ref concrete_opaque_types,
             ref upvar_list,
+            ref generator_interior_types,
 
         } = *self;
 
@@ -773,6 +807,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckTables<'tcx> {
             free_region_map.hash_stable(hcx, hasher);
             concrete_opaque_types.hash_stable(hcx, hasher);
             upvar_list.hash_stable(hcx, hasher);
+            generator_interior_types.hash_stable(hcx, hasher);
         })
     }
 }

src/librustc/ty/mod.rs

@@ -75,7 +75,7 @@ pub use self::binding::BindingMode;
 pub use self::binding::BindingMode::*;
 
 pub use self::context::{TyCtxt, FreeRegionInfo, AllArenas, tls, keep_local};
-pub use self::context::{Lift, TypeckTables, CtxtInterners, GlobalCtxt};
+pub use self::context::{Lift, GeneratorInteriorTypeCause, TypeckTables, CtxtInterners, GlobalCtxt};
 pub use self::context::{
     UserTypeAnnotationIndex, UserType, CanonicalUserType,
     CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, ResolvedOpaqueTy,

src/librustc_typeck/check/generator_interior.rs

@@ -14,7 +14,7 @@ use crate::util::nodemap::FxHashMap;
 
 struct InteriorVisitor<'a, 'tcx> {
     fcx: &'a FnCtxt<'a, 'tcx>,
-    types: FxHashMap<Ty<'tcx>, usize>,
+    types: FxHashMap<ty::GeneratorInteriorTypeCause<'tcx>, usize>,
     region_scope_tree: &'tcx region::ScopeTree,
     expr_count: usize,
     kind: hir::GeneratorKind,
@@ -83,7 +83,12 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
             } else {
                 // Map the type to the number of types added before it
                 let entries = self.types.len();
-                self.types.entry(&ty).or_insert(entries);
+                let scope_span = scope.map(|s| s.span(self.fcx.tcx, self.region_scope_tree));
+                self.types.entry(ty::GeneratorInteriorTypeCause {
+                    span: source_span,
+                    ty: &ty,
+                    scope_span
+                }).or_insert(entries);
             }
         } else {
             debug!("no type in expr = {:?}, count = {:?}, span = {:?}",
@@ -118,8 +123,12 @@ pub fn resolve_interior<'a, 'tcx>(
     // Sort types by insertion order
     types.sort_by_key(|t| t.1);
 
+    // Store the generator types and spans into the tables for this generator.
+    let interior_types = types.iter().cloned().map(|t| t.0).collect::<Vec<_>>();
+    visitor.fcx.inh.tables.borrow_mut().generator_interior_types = interior_types;
+
     // Extract type components
-    let type_list = fcx.tcx.mk_type_list(types.into_iter().map(|t| t.0));
+    let type_list = fcx.tcx.mk_type_list(types.into_iter().map(|t| (t.0).ty));
 
     // The types in the generator interior contain lifetimes local to the generator itself,
     // which should not be exposed outside of the generator. Therefore, we replace these

src/librustc_typeck/check/writeback.rs

@@ -58,6 +58,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         wbcx.visit_free_region_map();
         wbcx.visit_user_provided_tys();
         wbcx.visit_user_provided_sigs();
+        wbcx.visit_generator_interior_types();
 
         let used_trait_imports = mem::replace(
             &mut self.tables.borrow_mut().used_trait_imports,
@@ -430,6 +431,12 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
         }
     }
 
+    fn visit_generator_interior_types(&mut self) {
+        let fcx_tables = self.fcx.tables.borrow();
+        debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root);
+        self.tables.generator_interior_types = fcx_tables.generator_interior_types.clone();
+    }
+
     fn visit_opaque_types(&mut self, span: Span) {
         for (&def_id, opaque_defn) in self.fcx.opaque_types.borrow().iter() {
             let hir_id = self.tcx().hir().as_local_hir_id(def_id).unwrap();

src/libstd/future.rs

@@ -26,6 +26,7 @@ pub fn from_generator<T: Generator<Yield = ()>>(x: T) -> impl Future<Output = T:
 #[doc(hidden)]
 #[unstable(feature = "gen_future", issue = "50547")]
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
+#[cfg_attr(not(test), rustc_diagnostic_item = "gen_future")]
 struct GenFuture<T: Generator<Yield = ()>>(T);
 
 // We rely on the fact that async/await futures are immovable in order to create