Rollup merge of rust-lang#98365 - jyn514:improve-obligation-errors-re…

…view-comments, r=eholk

Address review comments from rust-lang#98259

It got approved so fast I didn't have time to make changes xD

r? `@eholk`

compiler/rustc_middle/src/ty/context.rs

@@ -74,7 +74,7 @@ use std::mem;
 use std::ops::{Bound, Deref};
 use std::sync::Arc;
 
-use super::RvalueScopes;
+use super::{ImplPolarity, RvalueScopes};
 
 pub trait OnDiskCache<'tcx>: rustc_data_structures::sync::Sync {
     /// Creates a new `OnDiskCache` instance from the serialized data in `data`.
@@ -2230,6 +2230,20 @@ impl<'tcx> TyCtxt<'tcx> {
         })
     }
 
+    /// Given a `ty`, return whether it's an `impl Future<...>`.
+    pub fn ty_is_opaque_future(self, ty: Ty<'_>) -> bool {
+        let ty::Opaque(def_id, _) = ty.kind() else { return false };
+        let future_trait = self.lang_items().future_trait().unwrap();
+
+        self.explicit_item_bounds(def_id).iter().any(|(predicate, _)| {
+            let ty::PredicateKind::Trait(trait_predicate) = predicate.kind().skip_binder() else {
+                return false;
+            };
+            trait_predicate.trait_ref.def_id == future_trait
+                && trait_predicate.polarity == ImplPolarity::Positive
+        })
+    }
+
     /// Computes the def-ids of the transitive supertraits of `trait_def_id`. This (intentionally)
     /// does not compute the full elaborated super-predicates but just the set of def-ids. It is used
     /// to identify which traits may define a given associated type to help avoid cycle errors.

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

@@ -8,7 +8,6 @@ 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::{
@@ -2396,19 +2395,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                     }
                 };
 
-                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
@@ -2434,13 +2420,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
 
                                 // 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);
+                                let is_future = tcx.ty_is_opaque_future(ty);
                                 debug!(
                                     ?obligated_types,
                                     ?is_future,
                                     "note_obligation_cause_code: check for async fn"
                                 );
-                                if opaque_ty_is_future(def_id)
+                                if is_future
                                     && obligated_types.last().map_or(false, |ty| match ty.kind() {
                                         ty::Opaque(last_def_id, _) => {
                                             tcx.parent(*last_def_id) == from_generator
@@ -2465,15 +2451,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                                 }
                                 err.note(msg.trim_end_matches(", "))
                             }
-                            ty::Generator(def_id, _, movability) => {
+                            ty::Generator(def_id, _, _) => {
                                 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"
-                                };
+                                let kind = tcx.generator_kind(def_id).unwrap();
                                 err.span_note(
                                     sp,
                                     &format!("required because it's used within this {}", kind),

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

@@ -42,7 +42,7 @@ 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's used within this async block
+note: required because it's used within this `async fn` body
   --> $DIR/issue-68112.rs:47:31
    |
 LL | async fn ready2<T>(t: T) -> T { t }
@@ -53,7 +53,7 @@ note: required because it appears within the type `impl Future<Output = Arc<RefC
 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
+note: required because it's used within this `async` block
   --> $DIR/issue-68112.rs:55:26
    |
 LL |       let send_fut = async {

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

@@ -14,7 +14,7 @@ LL |           baz(|| async{
 LL | |             foo(tx.clone());
 LL | |         }).await;
    | |_________^
-note: required because it's used within this async block
+note: required because it's used within this `async fn` body
   --> $DIR/issue-70935-complex-spans.rs:9:67
    |
 LL |   async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
@@ -23,7 +23,7 @@ LL | |
 LL | | }
    | |_^
    = note: required because it captures the following types: `ResumeTy`, `impl Future<Output = ()>`, `()`
-note: required because it's used within this async block
+note: required because it's used within this `async` block
   --> $DIR/issue-70935-complex-spans.rs:23:16
    |
 LL |       async move {

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

@@ -7,7 +7,7 @@
 use std::future::Future;
 
 async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
-//[drop_tracking]~^ within this async block
+//[drop_tracking]~^ within this `async fn` body
 }
 
 fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
@@ -21,7 +21,7 @@ fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
     //[drop_tracking]~|  NOTE: in this expansion
     //[drop_tracking]~|  NOTE: in this expansion
     async move {
-    //[drop_tracking]~^ within this async block
+    //[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

@@ -26,7 +26,7 @@ impl Drop for NotSend {
 impl !Send for NotSend {}
 
 async fn foo() {
-//~^ NOTE used within this async block
+//~^ NOTE used within this `async fn` body
 //~| NOTE within this `impl Future
     let mut x = (NotSend {},);
     drop(x.0);

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

@@ -12,7 +12,7 @@ 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 captures the following types: `ResumeTy`, `(NotSend,)`, `impl Future<Output = ()>`, `()`
-note: required because it's used within this async block
+note: required because it's used within this `async fn` body
   --> $DIR/partial-drop-partial-reinit.rs:28:16
    |
 LL |   async fn foo() {