Rollup merge of rust-lang#98259 - jyn514:improve-obligation-errors, r…

…=estebank

Greatly improve error reporting for futures and generators in `note_obligation_cause_code`

Most futures don't go through this code path, because they're caught by
`maybe_note_obligation_cause_for_async_await`. But all generators do,
and `maybe_note` is imperfect and doesn't catch all futures. Improve the error message for those it misses.

At some point, we may want to consider unifying this with the code for `maybe_note_async_await`,
so that `async_await` notes all parent constraints, and `note_obligation` can point to yield points.
But both functions are quite complicated, and it's not clear to me how to combine them;
this seems like a good incremental improvement.

Helps with rust-lang#97332.

r? `@estebank` cc `@eholk` `@compiler-errors`

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -8,6 +8,7 @@ use crate::infer::InferCtxt;
 use crate::traits::normalize_to;
 
 use hir::HirId;
+use rustc_ast::Movability;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_errors::{
@@ -2386,24 +2387,104 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                     {
                         let parent_trait_ref =
                             self.resolve_vars_if_possible(data.parent_trait_pred);
-                        let ty = parent_trait_ref.skip_binder().self_ty();
-                        matches!(ty.kind(), ty::Generator(..))
-                            || matches!(ty.kind(), ty::Closure(..))
+                        let nested_ty = parent_trait_ref.skip_binder().self_ty();
+                        matches!(nested_ty.kind(), ty::Generator(..))
+                            || matches!(nested_ty.kind(), ty::Closure(..))
                     } else {
                         false
                     }
                 };
 
+                let future_trait = self.tcx.lang_items().future_trait().unwrap();
+                let opaque_ty_is_future = |def_id| {
+                    self.tcx.explicit_item_bounds(def_id).iter().any(|(predicate, _)| {
+                        if let ty::PredicateKind::Trait(trait_predicate) =
+                            predicate.kind().skip_binder()
+                        {
+                            trait_predicate.trait_ref.def_id == future_trait
+                        } else {
+                            false
+                        }
+                    })
+                };
+
+                let from_generator = tcx.lang_items().from_generator_fn().unwrap();
+
                 // Don't print the tuple of capture types
-                if !is_upvar_tys_infer_tuple {
-                    let msg = format!("required because it appears within the type `{}`", ty);
-                    match ty.kind() {
-                        ty::Adt(def, _) => match self.tcx.opt_item_ident(def.did()) {
-                            Some(ident) => err.span_note(ident.span, &msg),
-                            None => err.note(&msg),
-                        },
-                        _ => err.note(&msg),
-                    };
+                'print: {
+                    if !is_upvar_tys_infer_tuple {
+                        let msg = format!("required because it appears within the type `{}`", ty);
+                        match ty.kind() {
+                            ty::Adt(def, _) => {
+                                // `gen_future` is used in all async functions; it doesn't add any additional info.
+                                if self.tcx.is_diagnostic_item(sym::gen_future, def.did()) {
+                                    break 'print;
+                                }
+                                match self.tcx.opt_item_ident(def.did()) {
+                                    Some(ident) => err.span_note(ident.span, &msg),
+                                    None => err.note(&msg),
+                                }
+                            }
+                            ty::Opaque(def_id, _) => {
+                                // Avoid printing the future from `core::future::from_generator`, it's not helpful
+                                if tcx.parent(*def_id) == from_generator {
+                                    break 'print;
+                                }
+
+                                // If the previous type is `from_generator`, this is the future generated by the body of an async function.
+                                // Avoid printing it twice (it was already printed in the `ty::Generator` arm below).
+                                let is_future = opaque_ty_is_future(def_id);
+                                debug!(
+                                    ?obligated_types,
+                                    ?is_future,
+                                    "note_obligation_cause_code: check for async fn"
+                                );
+                                if opaque_ty_is_future(def_id)
+                                    && obligated_types.last().map_or(false, |ty| match ty.kind() {
+                                        ty::Opaque(last_def_id, _) => {
+                                            tcx.parent(*last_def_id) == from_generator
+                                        }
+                                        _ => false,
+                                    })
+                                {
+                                    break 'print;
+                                }
+                                err.span_note(self.tcx.def_span(def_id), &msg)
+                            }
+                            ty::GeneratorWitness(bound_tys) => {
+                                use std::fmt::Write;
+
+                                // FIXME: this is kind of an unusual format for rustc, can we make it more clear?
+                                // Maybe we should just remove this note altogether?
+                                // FIXME: only print types which don't meet the trait requirement
+                                let mut msg =
+                                    "required because it captures the following types: ".to_owned();
+                                for ty in bound_tys.skip_binder() {
+                                    write!(msg, "`{}`, ", ty).unwrap();
+                                }
+                                err.note(msg.trim_end_matches(", "))
+                            }
+                            ty::Generator(def_id, _, movability) => {
+                                let sp = self.tcx.def_span(def_id);
+
+                                // Special-case this to say "async block" instead of `[static generator]`.
+                                let kind = if *movability == Movability::Static {
+                                    "async block"
+                                } else {
+                                    "generator"
+                                };
+                                err.span_note(
+                                    sp,
+                                    &format!("required because it's used within this {}", kind),
+                                )
+                            }
+                            ty::Closure(def_id, _) => err.span_note(
+                                self.tcx.def_span(def_id),
+                                &format!("required because it's used within this closure"),
+                            ),
+                            _ => err.note(&msg),
+                        };
+                    }
                 }
 
                 obligated_types.push(ty);

src/test/ui/async-await/issue-68112.stderr

@@ -42,15 +42,27 @@ LL |     require_send(send_fut);
    |
    = help: the trait `Sync` is not implemented for `RefCell<i32>`
    = note: required because of the requirements on the impl of `Send` for `Arc<RefCell<i32>>`
-   = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:47:31: 47:36]`
-   = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:47:31: 47:36]>`
-   = note: required because it appears within the type `impl Future<Output = Arc<RefCell<i32>>>`
-   = note: required because it appears within the type `impl Future<Output = Arc<RefCell<i32>>>`
-   = note: required because it appears within the type `impl Future<Output = Arc<RefCell<i32>>>`
-   = note: required because it appears within the type `{ResumeTy, impl Future<Output = Arc<RefCell<i32>>>, (), i32, Ready<i32>}`
-   = note: required because it appears within the type `[static generator@$DIR/issue-68112.rs:55:26: 59:6]`
-   = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/issue-68112.rs:55:26: 59:6]>`
-   = note: required because it appears within the type `impl Future<Output = ()>`
+note: required because it's used within this async block
+  --> $DIR/issue-68112.rs:47:31
+   |
+LL | async fn ready2<T>(t: T) -> T { t }
+   |                               ^^^^^
+note: required because it appears within the type `impl Future<Output = Arc<RefCell<i32>>>`
+  --> $DIR/issue-68112.rs:48:31
+   |
+LL | fn make_non_send_future2() -> impl Future<Output = Arc<RefCell<i32>>> {
+   |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: required because it captures the following types: `ResumeTy`, `impl Future<Output = Arc<RefCell<i32>>>`, `()`, `i32`, `Ready<i32>`
+note: required because it's used within this async block
+  --> $DIR/issue-68112.rs:55:26
+   |
+LL |       let send_fut = async {
+   |  __________________________^
+LL | |         let non_send_fut = make_non_send_future2();
+LL | |         let _ = non_send_fut.await;
+LL | |         ready(0).await;
+LL | |     };
+   | |_____^
 note: required by a bound in `require_send`
   --> $DIR/issue-68112.rs:11:25
    |

src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr

@@ -0,0 +1,40 @@
+error[E0277]: `Sender<i32>` cannot be shared between threads safely
+  --> $DIR/issue-70935-complex-spans.rs:13:45
+   |
+LL | fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
+   |                                             ^^^^^^^^^^^^^^^^^^ `Sender<i32>` cannot be shared between threads safely
+   |
+   = help: the trait `Sync` is not implemented for `Sender<i32>`
+   = note: required because of the requirements on the impl of `Send` for `&Sender<i32>`
+note: required because it's used within this closure
+  --> $DIR/issue-70935-complex-spans.rs:25:13
+   |
+LL |           baz(|| async{
+   |  _____________^
+LL | |             foo(tx.clone());
+LL | |         }).await;
+   | |_________^
+note: required because it's used within this async block
+  --> $DIR/issue-70935-complex-spans.rs:9:67
+   |
+LL |   async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
+   |  ___________________________________________________________________^
+LL | |
+LL | | }
+   | |_^
+   = note: required because it captures the following types: `ResumeTy`, `impl Future<Output = ()>`, `()`
+note: required because it's used within this async block
+  --> $DIR/issue-70935-complex-spans.rs:23:16
+   |
+LL |       async move {
+   |  ________________^
+LL | |
+LL | |         baz(|| async{
+LL | |             foo(tx.clone());
+LL | |         }).await;
+LL | |     }
+   | |_____^
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.

src/test/ui/async-await/issue-70935-complex-spans.stderr → src/test/ui/async-await/issue-70935-complex-spans.normal.stderr

@@ -1,22 +1,22 @@
 error: future cannot be sent between threads safely
-  --> $DIR/issue-70935-complex-spans.rs:10:45
+  --> $DIR/issue-70935-complex-spans.rs:13:45
    |
 LL | fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
    |                                             ^^^^^^^^^^^^^^^^^^ future created by async block is not `Send`
    |
    = help: the trait `Sync` is not implemented for `Sender<i32>`
 note: future is not `Send` as this value is used across an await
-  --> $DIR/issue-70935-complex-spans.rs:15:11
+  --> $DIR/issue-70935-complex-spans.rs:27:11
    |
 LL |           baz(|| async{
    |  _____________-
 LL | |             foo(tx.clone());
 LL | |         }).await;
    | |         - ^^^^^^ await occurs here, with the value maybe used later
    | |_________|
-   |           has type `[closure@$DIR/issue-70935-complex-spans.rs:13:13: 15:10]` which is not `Send`
+   |           has type `[closure@$DIR/issue-70935-complex-spans.rs:25:13: 27:10]` which is not `Send`
 note: the value is later dropped here
-  --> $DIR/issue-70935-complex-spans.rs:15:17
+  --> $DIR/issue-70935-complex-spans.rs:27:17
    |
 LL |         }).await;
    |                 ^

src/test/ui/async-await/issue-70935-complex-spans.rs

@@ -1,16 +1,28 @@
 // edition:2018
+// revisions: normal drop_tracking
+// [drop_tracking]compile-flags:-Zdrop-tracking
 // #70935: Check if we do not emit snippet
 // with newlines which lead complex diagnostics.
 
 use std::future::Future;
 
 async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
+//[drop_tracking]~^ within this async block
 }
 
 fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
-    //~^ ERROR: future cannot be sent between threads safely
+    //[normal]~^ ERROR: future cannot be sent between threads safely
+    //[drop_tracking]~^^ ERROR: `Sender<i32>` cannot be shared
+    //[drop_tracking]~|  NOTE: cannot be shared
+    //[drop_tracking]~|  NOTE: requirements on the impl of `Send`
+    //[drop_tracking]~|  NOTE: captures the following types
+    //[drop_tracking]~|  NOTE: in this expansion
+    //[drop_tracking]~|  NOTE: in this expansion
+    //[drop_tracking]~|  NOTE: in this expansion
+    //[drop_tracking]~|  NOTE: in this expansion
     async move {
-        baz(|| async{
+    //[drop_tracking]~^ within this async block
+        baz(|| async{ //[drop_tracking]~ NOTE: used within this closure
             foo(tx.clone());
         }).await;
     }

src/test/ui/async-await/partial-drop-partial-reinit.rs

@@ -5,9 +5,15 @@
 fn main() {
     gimme_send(foo());
     //~^ ERROR cannot be sent between threads safely
+    //~| NOTE cannot be sent
+    //~| NOTE bound introduced by
+    //~| NOTE appears within the type
+    //~| NOTE captures the following types
 }
 
 fn gimme_send<T: Send>(t: T) {
+//~^ NOTE required by this bound
+//~| NOTE required by a bound
     drop(t);
 }
 
@@ -20,6 +26,8 @@ impl Drop for NotSend {
 impl !Send for NotSend {}
 
 async fn foo() {
+//~^ NOTE used within this async block
+//~| NOTE within this `impl Future
     let mut x = (NotSend {},);
     drop(x.0);
     x.0 = NotSend {};

src/test/ui/async-await/partial-drop-partial-reinit.stderr

@@ -11,13 +11,21 @@ LL | async fn foo() {
    |
    = help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `NotSend`
    = note: required because it appears within the type `(NotSend,)`
-   = note: required because it appears within the type `{ResumeTy, (NotSend,), impl Future<Output = ()>, ()}`
-   = note: required because it appears within the type `[static generator@$DIR/partial-drop-partial-reinit.rs:22:16: 27:2]`
-   = note: required because it appears within the type `std::future::from_generator::GenFuture<[static generator@$DIR/partial-drop-partial-reinit.rs:22:16: 27:2]>`
-   = note: required because it appears within the type `impl Future<Output = ()>`
-   = note: required because it appears within the type `impl Future<Output = ()>`
+   = note: required because it captures the following types: `ResumeTy`, `(NotSend,)`, `impl Future<Output = ()>`, `()`
+note: required because it's used within this async block
+  --> $DIR/partial-drop-partial-reinit.rs:28:16
+   |
+LL |   async fn foo() {
+   |  ________________^
+LL | |
+LL | |
+LL | |     let mut x = (NotSend {},);
+...  |
+LL | |     bar().await;
+LL | | }
+   | |_^
 note: required by a bound in `gimme_send`
-  --> $DIR/partial-drop-partial-reinit.rs:10:18
+  --> $DIR/partial-drop-partial-reinit.rs:14:18
    |
 LL | fn gimme_send<T: Send>(t: T) {
    |                  ^^^^ required by this bound in `gimme_send`

src/test/ui/closures/closure-move-sync.stderr

@@ -6,7 +6,15 @@ LL |     let t = thread::spawn(|| {
    |
    = help: the trait `Sync` is not implemented for `std::sync::mpsc::Receiver<()>`
    = note: required because of the requirements on the impl of `Send` for `&std::sync::mpsc::Receiver<()>`
-   = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:6:27: 9:6]`
+note: required because it's used within this closure
+  --> $DIR/closure-move-sync.rs:6:27
+   |
+LL |       let t = thread::spawn(|| {
+   |  ___________________________^
+LL | |         recv.recv().unwrap();
+LL | |
+LL | |     });
+   | |_____^
 note: required by a bound in `spawn`
   --> $SRC_DIR/std/src/thread/mod.rs:LL:COL
    |
@@ -21,7 +29,11 @@ LL |     thread::spawn(|| tx.send(()).unwrap());
    |
    = help: the trait `Sync` is not implemented for `Sender<()>`
    = note: required because of the requirements on the impl of `Send` for `&Sender<()>`
-   = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:18:19: 18:42]`
+note: required because it's used within this closure
+  --> $DIR/closure-move-sync.rs:18:19
+   |
+LL |     thread::spawn(|| tx.send(()).unwrap());
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^
 note: required by a bound in `spawn`
   --> $SRC_DIR/std/src/thread/mod.rs:LL:COL
    |

src/test/ui/generator/issue-68112.rs

@@ -20,6 +20,10 @@ pub fn make_gen1<T>(t: T) -> Ready<T> {
 }
 
 fn require_send(_: impl Send) {}
+//~^ NOTE required by a bound
+//~| NOTE required by a bound
+//~| NOTE required by this bound
+//~| NOTE required by this bound
 
 fn make_non_send_generator() -> impl Generator<Return = Arc<RefCell<i32>>> {
     make_gen1(Arc::new(RefCell::new(0)))
@@ -28,29 +32,39 @@ fn make_non_send_generator() -> impl Generator<Return = Arc<RefCell<i32>>> {
 fn test1() {
     let send_gen = || {
         let _non_send_gen = make_non_send_generator();
+        //~^ NOTE not `Send`
         yield;
-    };
+        //~^ NOTE yield occurs here
+        //~| NOTE value is used across a yield
+    }; //~ NOTE later dropped here
     require_send(send_gen);
     //~^ ERROR generator cannot be sent between threads
+    //~| NOTE not `Send`
 }
 
 pub fn make_gen2<T>(t: T) -> impl Generator<Return = T> {
-    || {
+//~^ NOTE appears within the type
+//~| NOTE expansion of desugaring
+    || { //~ NOTE used within this generator
         yield;
         t
     }
 }
-fn make_non_send_generator2() -> impl Generator<Return = Arc<RefCell<i32>>> {
+fn make_non_send_generator2() -> impl Generator<Return = Arc<RefCell<i32>>> { //~ NOTE appears within the type
+//~^ NOTE expansion of desugaring
     make_gen2(Arc::new(RefCell::new(0)))
 }
 
 fn test2() {
-    let send_gen = || {
+    let send_gen = || { //~ NOTE used within this generator
         let _non_send_gen = make_non_send_generator2();
         yield;
     };
     require_send(send_gen);
     //~^ ERROR `RefCell<i32>` cannot be shared between threads safely
+    //~| NOTE `RefCell<i32>` cannot be shared between threads safely
+    //~| NOTE requirements on the impl
+    //~| NOTE captures the following types
 }
 
 fn main() {}