Note inference failures using `?` conversion

10 files changed, 239 insertions, 44 deletions

diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
index d1d6cb43fc7..7b0a91986b3 100644
--- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs
+++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
@@ -69,7 +69,7 @@ use rustc_middle::ty::{
     subst::{Subst, SubstsRef},
     Region, Ty, TyCtxt, TypeFoldable,
 };
-use rustc_span::{BytePos, DesugaringKind, Pos, Span};
+use rustc_span::{sym, BytePos, DesugaringKind, Pos, Span};
 use rustc_target::spec::abi;
 use std::ops::ControlFlow;
 use std::{cmp, fmt};
@@ -2282,6 +2282,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         self.note_region_origin(&mut err, &sub_origin);
         err.emit();
     }
+
+    /// Determine whether an error associated with the given span and definition
+    /// should be treated as being caused by the implicit `From` conversion
+    /// within `?` desugaring.
+    pub fn is_try_conversion(&self, span: Span, trait_def_id: DefId) -> bool {
+        span.is_desugaring(DesugaringKind::QuestionMark)
+            && self.tcx.is_diagnostic_item(sym::from_trait, trait_def_id)
+    }
 }
 
 impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
diff --git a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
index e097264ec8a..0e236853ae8 100644
--- a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
+++ b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
@@ -3,6 +3,7 @@ use crate::infer::InferCtxt;
 use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder};
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Namespace};
+use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
 use rustc_hir::{Body, Expr, ExprKind, FnRetTy, HirId, Local, Pat};
 use rustc_middle::hir::map::Map;
@@ -25,6 +26,7 @@ struct FindHirNodeVisitor<'a, 'tcx> {
     found_closure: Option<&'tcx Expr<'tcx>>,
     found_method_call: Option<&'tcx Expr<'tcx>>,
     found_exact_method_call: Option<&'tcx Expr<'tcx>>,
+    found_use_diagnostic: Option<UseDiagnostic<'tcx>>,
 }
 
 impl<'a, 'tcx> FindHirNodeVisitor<'a, 'tcx> {
@@ -39,34 +41,43 @@ impl<'a, 'tcx> FindHirNodeVisitor<'a, 'tcx> {
             found_closure: None,
             found_method_call: None,
             found_exact_method_call: None,
+            found_use_diagnostic: None,
         }
     }
 
-    fn node_ty_contains_target(&mut self, hir_id: HirId) -> Option<Ty<'tcx>> {
-        self.infcx
-            .in_progress_typeck_results
-            .and_then(|typeck_results| typeck_results.borrow().node_type_opt(hir_id))
-            .map(|ty| self.infcx.resolve_vars_if_possible(ty))
-            .filter(|ty| {
-                ty.walk().any(|inner| {
-                    inner == self.target
-                        || match (inner.unpack(), self.target.unpack()) {
-                            (GenericArgKind::Type(inner_ty), GenericArgKind::Type(target_ty)) => {
-                                use ty::{Infer, TyVar};
-                                match (inner_ty.kind(), target_ty.kind()) {
-                                    (&Infer(TyVar(a_vid)), &Infer(TyVar(b_vid))) => self
-                                        .infcx
-                                        .inner
-                                        .borrow_mut()
-                                        .type_variables()
-                                        .sub_unified(a_vid, b_vid),
-                                    _ => false,
-                                }
+    fn node_type_opt(&self, hir_id: HirId) -> Option<Ty<'tcx>> {
+        self.infcx.in_progress_typeck_results?.borrow().node_type_opt(hir_id)
+    }
+
+    fn node_ty_contains_target(&self, hir_id: HirId) -> Option<Ty<'tcx>> {
+        self.node_type_opt(hir_id).map(|ty| self.infcx.resolve_vars_if_possible(ty)).filter(|ty| {
+            ty.walk().any(|inner| {
+                inner == self.target
+                    || match (inner.unpack(), self.target.unpack()) {
+                        (GenericArgKind::Type(inner_ty), GenericArgKind::Type(target_ty)) => {
+                            use ty::{Infer, TyVar};
+                            match (inner_ty.kind(), target_ty.kind()) {
+                                (&Infer(TyVar(a_vid)), &Infer(TyVar(b_vid))) => self
+                                    .infcx
+                                    .inner
+                                    .borrow_mut()
+                                    .type_variables()
+                                    .sub_unified(a_vid, b_vid),
+                                _ => false,
                             }
-                            _ => false,
                         }
-                })
+                        _ => false,
+                    }
             })
+        })
+    }
+
+    /// Determine whether the expression, assumed to be the callee within a `Call`,
+    /// corresponds to the `From::from` emitted in desugaring of the `?` operator.
+    fn is_try_conversion(&self, callee: &Expr<'tcx>) -> bool {
+        self.infcx
+            .trait_def_from_hir_fn(callee.hir_id)
+            .map_or(false, |def_id| self.infcx.is_try_conversion(callee.span, def_id))
     }
 }
 
@@ -119,10 +130,23 @@ impl<'a, 'tcx> Visitor<'tcx> for FindHirNodeVisitor<'a, 'tcx> {
         // are handled specially, but instead they should be handled in `annotate_method_call`,
         // which currently doesn't work because this evaluates to `false` for const arguments.
         // See https://github.com/rust-lang/rust/pull/77758 for more details.
-        if self.node_ty_contains_target(expr.hir_id).is_some() {
+        if let Some(ty) = self.node_ty_contains_target(expr.hir_id) {
             match expr.kind {
                 ExprKind::Closure(..) => self.found_closure = Some(&expr),
                 ExprKind::MethodCall(..) => self.found_method_call = Some(&expr),
+
+                // If the given expression falls within the target span and is a
+                // `From::from(e)` call emitted during desugaring of the `?` operator,
+                // extract the types inferred before and after the call
+                ExprKind::Call(callee, [arg])
+                    if self.target_span.contains(expr.span)
+                        && self.found_use_diagnostic.is_none()
+                        && self.is_try_conversion(callee) =>
+                {
+                    self.found_use_diagnostic = self.node_type_opt(arg.hir_id).map(|pre_ty| {
+                        UseDiagnostic::TryConversion { pre_ty, post_ty: ty, span: callee.span }
+                    });
+                }
                 _ => {}
             }
         }
@@ -130,6 +154,67 @@ impl<'a, 'tcx> Visitor<'tcx> for FindHirNodeVisitor<'a, 'tcx> {
     }
 }
 
+/// An observation about the use site of a type to be emitted as an additional
+/// note in an inference failure error.
+enum UseDiagnostic<'tcx> {
+    /// Records the types inferred before and after `From::from` is called on the
+    /// error value within the desugaring of the `?` operator.
+    TryConversion { pre_ty: Ty<'tcx>, post_ty: Ty<'tcx>, span: Span },
+}
+
+impl UseDiagnostic<'_> {
+    /// Return a descriptor of the value at the use site
+    fn descr(&self) -> &'static str {
+        match self {
+            Self::TryConversion { .. } => "`?` error",
+        }
+    }
+
+    /// Return a descriptor of the type at the use site
+    fn type_descr(&self) -> &'static str {
+        match self {
+            Self::TryConversion { .. } => "`?` error type",
+        }
+    }
+
+    fn applies_to(&self, span: Span) -> bool {
+        match *self {
+            // In some cases the span for an inference failure due to try
+            // conversion contains the antecedent expression as well as the `?`
+            Self::TryConversion { span: s, .. } => span.contains(s) && span.hi() == s.hi(),
+        }
+    }
+
+    fn attach_note(&self, err: &mut DiagnosticBuilder<'_>) {
+        match *self {
+            Self::TryConversion { pre_ty, post_ty, .. } => {
+                let pre_ty = pre_ty.to_string();
+                let post_ty = post_ty.to_string();
+
+                let intro = "the `?` operation implicitly converts the error value";
+
+                let msg = match (pre_ty.as_str(), post_ty.as_str()) {
+                    ("_", "_") => format!("{} using the `From` trait", intro),
+                    (_, "_") => {
+                        format!("{} into a type implementing `From<{}>`", intro, pre_ty)
+                    }
+                    ("_", _) => {
+                        format!("{} into `{}` using the `From` trait", intro, post_ty)
+                    }
+                    (_, _) => {
+                        format!(
+                            "{} into `{}` using its implementation of `From<{}>`",
+                            intro, post_ty, pre_ty
+                        )
+                    }
+                };
+
+                err.note(&msg);
+            }
+        }
+    }
+}
+
 /// Suggest giving an appropriate return type to a closure expression.
 fn closure_return_type_suggestion(
     span: Span,
@@ -139,6 +224,7 @@ fn closure_return_type_suggestion(
     descr: &str,
     name: &str,
     ret: &str,
+    use_diag: Option<&UseDiagnostic<'_>>,
     parent_name: Option<String>,
     parent_descr: Option<&str>,
 ) {
@@ -160,7 +246,15 @@ fn closure_return_type_suggestion(
     );
     err.span_label(
         span,
-        InferCtxt::cannot_infer_msg("type", &name, &descr, parent_name, parent_descr),
+        InferCtxt::cannot_infer_msg(
+            span,
+            "type",
+            &name,
+            &descr,
+            use_diag,
+            parent_name,
+            parent_descr,
+        ),
     );
 }
 
@@ -420,7 +514,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
 
         // When `arg_data.name` corresponds to a type argument, show the path of the full type we're
         // trying to infer. In the following example, `ty_msg` contains
-        // " in `std::result::Result<i32, E>`":
+        // " for `std::result::Result<i32, E>`":
         // ```
         // error[E0282]: type annotations needed for `std::result::Result<i32, E>`
         //  --> file.rs:L:CC
@@ -438,6 +532,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             error_code,
         );
 
+        let use_diag = local_visitor.found_use_diagnostic.as_ref();
+        if let Some(use_diag) = use_diag {
+            if use_diag.applies_to(err_span) {
+                use_diag.attach_note(&mut err);
+            }
+        }
+
         let suffix = match local_visitor.found_node_ty {
             Some(ty) if ty.is_closure() => {
                 let substs =
@@ -460,6 +561,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                         &arg_data.description,
                         &arg_data.name,
                         &ret,
+                        use_diag,
                         arg_data.parent_name,
                         arg_data.parent_description,
                     );
@@ -634,9 +736,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             err.span_label(
                 span,
                 InferCtxt::cannot_infer_msg(
+                    span,
                     kind_str,
                     &arg_data.name,
                     &arg_data.description,
+                    use_diag,
                     arg_data.parent_name,
                     arg_data.parent_description,
                 ),
@@ -646,6 +750,20 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         err
     }
 
+    fn trait_def_from_hir_fn(&self, hir_id: hir::HirId) -> Option<DefId> {
+        // The DefId will be the method's trait item ID unless this is an inherent impl
+        if let Some((DefKind::AssocFn, def_id)) =
+            self.in_progress_typeck_results?.borrow().type_dependent_def(hir_id)
+        {
+            return self
+                .tcx
+                .parent(def_id)
+                .filter(|&parent_def_id| self.tcx.is_trait(parent_def_id));
+        }
+
+        None
+    }
+
     /// If the `FnSig` for the method call can be found and type arguments are identified as
     /// needed, suggest annotating the call, otherwise point out the resulting type of the call.
     fn annotate_method_call(
@@ -711,9 +829,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         err.span_label(
             span,
             InferCtxt::cannot_infer_msg(
+                span,
                 "type",
                 &data.name,
                 &data.description,
+                None,
                 data.parent_name,
                 data.parent_description,
             ),
@@ -722,16 +842,24 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     }
 
     fn cannot_infer_msg(
+        span: Span,
         kind_str: &str,
         type_name: &str,
         descr: &str,
+        use_diag: Option<&UseDiagnostic<'_>>,
         parent_name: Option<String>,
         parent_descr: Option<&str>,
     ) -> String {
+        let use_diag = use_diag.filter(|d| d.applies_to(span));
+
         if type_name == "_" {
-            format!("cannot infer {}", kind_str)
+            if let Some(use_diag) = use_diag {
+                format!("cannot infer {} of {}", kind_str, use_diag.descr())
+            } else {
+                format!("cannot infer {}", kind_str)
+            }
         } else {
-            let parent_desc = if let Some(parent_name) = parent_name {
+            let extra_descr = if let Some(parent_name) = parent_name {
                 let parent_type_descr = if let Some(parent_descr) = parent_descr {
                     format!(" the {}", parent_descr)
                 } else {
@@ -739,8 +867,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                 };
 
                 format!(" declared on{} `{}`", parent_type_descr, parent_name)
+            } else if let Some(use_diag) = use_diag {
+                format!(" in {}", use_diag.type_descr())
             } else {
-                "".to_string()
+                "".into()
             };
 
             // FIXME: We really shouldn't be dealing with strings here
@@ -749,7 +879,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             // For example: "cannot infer type for type parameter `T`"
             format!(
                 "cannot infer {} {} {} `{}`{}",
-                kind_str, preposition, descr, type_name, parent_desc
+                kind_str, preposition, descr, type_name, extra_descr
             )
         }
     }
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
index a439bb892f8..0186d164a4c 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
@@ -280,18 +280,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                         let OnUnimplementedNote { message, label, note, enclosing_scope } =
                             self.on_unimplemented_note(trait_ref, obligation);
                         let have_alt_message = message.is_some() || label.is_some();
-                        let is_try = self
-                            .tcx
-                            .sess
-                            .source_map()
-                            .span_to_snippet(span)
-                            .map(|s| &s == "?")
-                            .unwrap_or(false);
-                        let is_from = self.tcx.get_diagnostic_item(sym::from_trait)
-                            == Some(trait_ref.def_id());
+                        let is_try_conversion = self.is_try_conversion(span, trait_ref.def_id());
                         let is_unsize =
                             { Some(trait_ref.def_id()) == self.tcx.lang_items().unsize_trait() };
-                        let (message, note) = if is_try && is_from {
+                        let (message, note) = if is_try_conversion {
                             (
                                 Some(format!(
                                     "`?` couldn't convert the error to `{}`",
@@ -319,7 +311,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                             ))
                         );
 
-                        if is_try && is_from {
+                        if is_try_conversion {
                             let none_error = self
                                 .tcx
                                 .get_diagnostic_item(sym::none_error)
diff --git a/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr
index 2f630c2c9ad..de15d472d05 100644
--- a/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr
+++ b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr
@@ -13,7 +13,9 @@ error[E0282]: type annotations needed for `impl Future`
 LL |     let fut = async {
    |         --- consider giving `fut` the explicit type `impl Future`, with the type parameters specified
 LL |         make_unit()?;
-   |                    ^ cannot infer type
+   |                    ^ cannot infer type of `?` error
+   |
+   = note: the `?` operation implicitly converts the error value into a type implementing `From<std::io::Error>`
 
 error: aborting due to previous error; 1 warning emitted
 
diff --git a/src/test/ui/inference/cannot-infer-async.stderr b/src/test/ui/inference/cannot-infer-async.stderr
index 92a9045f6db..d5cccc7a948 100644
--- a/src/test/ui/inference/cannot-infer-async.stderr
+++ b/src/test/ui/inference/cannot-infer-async.stderr
@@ -4,7 +4,9 @@ error[E0282]: type annotations needed
 LL |     let fut = async {
    |         --- consider giving `fut` a type
 LL |         make_unit()?;
-   |                    ^ cannot infer type
+   |                    ^ cannot infer type of `?` error
+   |
+   = note: the `?` operation implicitly converts the error value into a type implementing `From<std::io::Error>`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/inference/cannot-infer-closure-circular.rs b/src/test/ui/inference/cannot-infer-closure-circular.rs
new file mode 100644
index 00000000000..a3b957179b1
--- /dev/null
+++ b/src/test/ui/inference/cannot-infer-closure-circular.rs
@@ -0,0 +1,14 @@
+fn main() {
+    // Below we call the closure with its own return as the argument, unifying
+    // its inferred input and return types. We want to make sure that the generated
+    // error handles this gracefully, and in particular doesn't generate an extra
+    // note about the `?` operator in the closure body, which isn't relevant to
+    // the inference.
+    let x = |r| {
+        //~^ ERROR type annotations needed
+        let v = r?;
+        Ok(v)
+    };
+
+    let _ = x(x(Ok(())));
+}
diff --git a/src/test/ui/inference/cannot-infer-closure-circular.stderr b/src/test/ui/inference/cannot-infer-closure-circular.stderr
new file mode 100644
index 00000000000..5efb400a4c7
--- /dev/null
+++ b/src/test/ui/inference/cannot-infer-closure-circular.stderr
@@ -0,0 +1,9 @@
+error[E0282]: type annotations needed for `std::result::Result<(), E>`
+  --> $DIR/cannot-infer-closure-circular.rs:7:14
+   |
+LL |     let x = |r| {
+   |              ^ consider giving this closure parameter the explicit type `std::result::Result<(), E>`, with the type parameters specified
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0282`.
diff --git a/src/test/ui/inference/cannot-infer-closure.stderr b/src/test/ui/inference/cannot-infer-closure.stderr
index d5366e422db..c083f2b686e 100644
--- a/src/test/ui/inference/cannot-infer-closure.stderr
+++ b/src/test/ui/inference/cannot-infer-closure.stderr
@@ -2,8 +2,9 @@ error[E0282]: type annotations needed for the closure `fn((), ()) -> std::result
   --> $DIR/cannot-infer-closure.rs:3:15
    |
 LL |         Err(a)?;
-   |               ^ cannot infer type
+   |               ^ cannot infer type of `?` error
    |
+   = note: the `?` operation implicitly converts the error value into a type implementing `From<()>`
 help: give this closure an explicit return type without `_` placeholders
    |
 LL |     let x = |a: (), b: ()| -> std::result::Result<(), _> {
diff --git a/src/test/ui/inference/cannot-infer-partial-try-return.rs b/src/test/ui/inference/cannot-infer-partial-try-return.rs
new file mode 100644
index 00000000000..e1058e96cef
--- /dev/null
+++ b/src/test/ui/inference/cannot-infer-partial-try-return.rs
@@ -0,0 +1,22 @@
+struct QualifiedError<E>(E);
+
+impl<E, T> From<E> for QualifiedError<T>
+where
+    E: std::error::Error,
+    T: From<E>,
+{
+    fn from(e: E) -> QualifiedError<T> {
+        QualifiedError(e.into())
+    }
+}
+
+fn infallible() -> Result<(), std::convert::Infallible> {
+    Ok(())
+}
+
+fn main() {
+    let x = || -> Result<_, QualifiedError<_>> {
+        infallible()?; //~ERROR type annotations needed
+        Ok(())
+    };
+}
diff --git a/src/test/ui/inference/cannot-infer-partial-try-return.stderr b/src/test/ui/inference/cannot-infer-partial-try-return.stderr
new file mode 100644
index 00000000000..d4223bfc155
--- /dev/null
+++ b/src/test/ui/inference/cannot-infer-partial-try-return.stderr
@@ -0,0 +1,15 @@
+error[E0282]: type annotations needed for the closure `fn() -> std::result::Result<(), QualifiedError<_>>`
+  --> $DIR/cannot-infer-partial-try-return.rs:19:9
+   |
+LL |         infallible()?;
+   |         ^^^^^^^^^^^^^ cannot infer type of `?` error
+   |
+   = note: the `?` operation implicitly converts the error value into `QualifiedError<_>` using its implementation of `From<Infallible>`
+help: give this closure an explicit return type without `_` placeholders
+   |
+LL |     let x = || -> std::result::Result<(), QualifiedError<_>> {
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0282`.