Auto merge of #85596 - scottmcm:more-on-unimplemented, r=estebank

Extend `rustc_on_implemented` to improve more `?` error messages

`_Self` could match the generic definition; this adds that functionality for matching the generic definition of type parameters too.

Your advice welcome on the wording of all these messages, and which things belong in the message/label/note.

r? `@estebank`

5 files changed, 47 insertions, 15 deletions

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs
index 1ea34e5814e..0ca0245a203 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs
@@ -186,6 +186,15 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                 };
                 let name = param.name;
                 flags.push((name, Some(value)));
+
+                if let GenericParamDefKind::Type { .. } = param.kind {
+                    let param_ty = trait_ref.substs[param.index as usize].expect_ty();
+                    if let Some(def) = param_ty.ty_adt_def() {
+                        // We also want to be able to select the parameter's
+                        // original signature with no type arguments resolved
+                        flags.push((name, Some(self.tcx.type_of(def.did).to_string())));
+                    }
+                }
             }
 
             if let Some(true) = self_ty.ty_adt_def().map(|def| def.did.is_local()) {
diff --git a/library/core/src/ops/try_trait.rs b/library/core/src/ops/try_trait.rs
index 87044ed2fce..1d9bc452618 100644
--- a/library/core/src/ops/try_trait.rs
+++ b/library/core/src/ops/try_trait.rs
@@ -259,6 +259,18 @@ pub trait Try: FromResidual {
             from_method = "from_residual",
             from_desugaring = "QuestionMark",
             _Self = "std::option::Option<T>",
+            R = "std::result::Result<T, E>",
+        ),
+        message = "the `?` operator can only be used on `Option`s, not `Result`s, \
+            in {ItemContext} that returns `Option`",
+        label = "use `.ok()?` if you want to discard the `{R}` error information",
+        enclosing_scope = "this function returns an `Option`"
+    ),
+    on(
+        all(
+            from_method = "from_residual",
+            from_desugaring = "QuestionMark",
+            _Self = "std::option::Option<T>",
         ),
         // `Option`-in-`Option` always works, as there's only one possible
         // residual, so this can also be phrased strongly.
@@ -272,9 +284,10 @@ pub trait Try: FromResidual {
             from_method = "from_residual",
             from_desugaring = "QuestionMark",
             _Self = "std::ops::ControlFlow<B, C>",
+            R = "std::ops::ControlFlow<B, C>",
         ),
-        message = "the `?` operator can only be used on `ControlFlow<B, _>`s \
-            in {ItemContext} that returns `ControlFlow<B, _>`",
+        message = "the `?` operator in {ItemContext} that returns `ControlFlow<B, _>` \
+            can only be used on other `ControlFlow<B, _>`s (with the same Break type)",
         label = "this `?` produces `{R}`, which is incompatible with `{Self}`",
         enclosing_scope = "this function returns a `ControlFlow`",
         note = "unlike `Result`, there's no `From`-conversion performed for `ControlFlow`"
@@ -282,6 +295,18 @@ pub trait Try: FromResidual {
     on(
         all(
             from_method = "from_residual",
+            from_desugaring = "QuestionMark",
+            _Self = "std::ops::ControlFlow<B, C>",
+            // `R` is not a `ControlFlow`, as that case was matched previously
+        ),
+        message = "the `?` operator can only be used on `ControlFlow`s \
+            in {ItemContext} that returns `ControlFlow`",
+        label = "this `?` produces `{R}`, which is incompatible with `{Self}`",
+        enclosing_scope = "this function returns a `ControlFlow`",
+    ),
+    on(
+        all(
+            from_method = "from_residual",
             from_desugaring = "QuestionMark"
         ),
         message = "the `?` operator can only be used in {ItemContext} \
diff --git a/src/test/ui/try-trait/bad-interconversion.rs b/src/test/ui/try-trait/bad-interconversion.rs
index 87585822f57..385f5510fb4 100644
--- a/src/test/ui/try-trait/bad-interconversion.rs
+++ b/src/test/ui/try-trait/bad-interconversion.rs
@@ -20,7 +20,7 @@ fn control_flow_to_result() -> Result<u64, String> {
 
 fn result_to_option() -> Option<u16> {
     Some(Err("hello")?)
-    //~^ ERROR the `?` operator can only be used on `Option`s in a function that returns `Option`
+    //~^ ERROR the `?` operator can only be used on `Option`s, not `Result`s, in a function that returns `Option`
 }
 
 fn control_flow_to_option() -> Option<u64> {
@@ -30,18 +30,18 @@ fn control_flow_to_option() -> Option<u64> {
 
 fn result_to_control_flow() -> ControlFlow<String> {
     ControlFlow::Continue(Err("hello")?)
-    //~^ ERROR the `?` operator can only be used on `ControlFlow<B, _>`s in a function that returns `ControlFlow<B, _>`
+    //~^ ERROR the `?` operator can only be used on `ControlFlow`s in a function that returns `ControlFlow`
 }
 
 fn option_to_control_flow() -> ControlFlow<u64> {
     Some(3)?;
-    //~^ ERROR the `?` operator can only be used on `ControlFlow<B, _>`s in a function that returns `ControlFlow<B, _>`
+    //~^ ERROR the `?` operator can only be used on `ControlFlow`s in a function that returns `ControlFlow`
     ControlFlow::Break(10)
 }
 
 fn control_flow_to_control_flow() -> ControlFlow<i64> {
     ControlFlow::Break(4_u8)?;
-    //~^ ERROR the `?` operator can only be used on `ControlFlow<B, _>`s in a function that returns `ControlFlow<B, _>`
+    //~^ ERROR the `?` operator in a function that returns `ControlFlow<B, _>` can only be used on other `ControlFlow<B, _>`s
     ControlFlow::Continue(())
 }
 
diff --git a/src/test/ui/try-trait/bad-interconversion.stderr b/src/test/ui/try-trait/bad-interconversion.stderr
index e396256de22..f5b315c2519 100644
--- a/src/test/ui/try-trait/bad-interconversion.stderr
+++ b/src/test/ui/try-trait/bad-interconversion.stderr
@@ -40,12 +40,12 @@ LL | | }
    = help: the trait `FromResidual<ControlFlow<{integer}, Infallible>>` is not implemented for `Result<u64, String>`
    = note: required by `from_residual`
 
-error[E0277]: the `?` operator can only be used on `Option`s in a function that returns `Option`
+error[E0277]: the `?` operator can only be used on `Option`s, not `Result`s, in a function that returns `Option`
   --> $DIR/bad-interconversion.rs:22:22
    |
 LL | / fn result_to_option() -> Option<u16> {
 LL | |     Some(Err("hello")?)
-   | |                      ^ this `?` produces `Result<Infallible, &str>`, which is incompatible with `Option<u16>`
+   | |                      ^ use `.ok()?` if you want to discard the `Result<Infallible, &str>` error information
 LL | |
 LL | | }
    | |_- this function returns an `Option`
@@ -66,7 +66,7 @@ LL | | }
    = help: the trait `FromResidual<ControlFlow<{integer}, Infallible>>` is not implemented for `Option<u64>`
    = note: required by `from_residual`
 
-error[E0277]: the `?` operator can only be used on `ControlFlow<B, _>`s in a function that returns `ControlFlow<B, _>`
+error[E0277]: the `?` operator can only be used on `ControlFlow`s in a function that returns `ControlFlow`
   --> $DIR/bad-interconversion.rs:32:39
    |
 LL | / fn result_to_control_flow() -> ControlFlow<String> {
@@ -77,10 +77,9 @@ LL | | }
    | |_- this function returns a `ControlFlow`
    |
    = help: the trait `FromResidual<Result<Infallible, &str>>` is not implemented for `ControlFlow<String>`
-   = note: unlike `Result`, there's no `From`-conversion performed for `ControlFlow`
    = note: required by `from_residual`
 
-error[E0277]: the `?` operator can only be used on `ControlFlow<B, _>`s in a function that returns `ControlFlow<B, _>`
+error[E0277]: the `?` operator can only be used on `ControlFlow`s in a function that returns `ControlFlow`
   --> $DIR/bad-interconversion.rs:37:12
    |
 LL | / fn option_to_control_flow() -> ControlFlow<u64> {
@@ -92,10 +91,9 @@ LL | | }
    | |_- this function returns a `ControlFlow`
    |
    = help: the trait `FromResidual<Option<Infallible>>` is not implemented for `ControlFlow<u64>`
-   = note: unlike `Result`, there's no `From`-conversion performed for `ControlFlow`
    = note: required by `from_residual`
 
-error[E0277]: the `?` operator can only be used on `ControlFlow<B, _>`s in a function that returns `ControlFlow<B, _>`
+error[E0277]: the `?` operator in a function that returns `ControlFlow<B, _>` can only be used on other `ControlFlow<B, _>`s (with the same Break type)
   --> $DIR/bad-interconversion.rs:43:29
    |
 LL | / fn control_flow_to_control_flow() -> ControlFlow<i64> {
diff --git a/src/test/ui/try-trait/option-to-result.stderr b/src/test/ui/try-trait/option-to-result.stderr
index 92087c2aba2..9f7d80d4f23 100644
--- a/src/test/ui/try-trait/option-to-result.stderr
+++ b/src/test/ui/try-trait/option-to-result.stderr
@@ -12,13 +12,13 @@ LL | | }
    = help: the trait `FromResidual<Option<Infallible>>` is not implemented for `Result<(), ()>`
    = note: required by `from_residual`
 
-error[E0277]: the `?` operator can only be used on `Option`s in a function that returns `Option`
+error[E0277]: the `?` operator can only be used on `Option`s, not `Result`s, in a function that returns `Option`
   --> $DIR/option-to-result.rs:11:6
    |
 LL | / fn test_option() -> Option<i32>{
 LL | |     let a:Result<i32, i32> = Ok(5);
 LL | |     a?;
-   | |      ^ this `?` produces `Result<Infallible, i32>`, which is incompatible with `Option<i32>`
+   | |      ^ use `.ok()?` if you want to discard the `Result<Infallible, i32>` error information
 LL | |     Some(5)
 LL | | }
    | |_- this function returns an `Option`