From 8be67998a1f488b386d2982b3ddeec65099ab14c Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sat, 22 May 2021 23:47:12 -0700 Subject: [PATCH] Extend rustc_on_implemented to improve a ?-on-ControlFlow error message --- .../error_reporting/on_unimplemented.rs | 9 ++++++ library/core/src/ops/try_trait.rs | 29 +++++++++++++++++-- src/test/ui/try-trait/bad-interconversion.rs | 8 ++--- .../ui/try-trait/bad-interconversion.stderr | 12 ++++---- src/test/ui/try-trait/option-to-result.stderr | 4 +-- 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 @@ fn on_unimplemented_note( }; 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 @@ -254,6 +254,18 @@ pub trait Try: FromResidual { label = "this `?` produces `{R}`, which is incompatible with `{Self}`", enclosing_scope = "this function returns a `Result`" ), + on( + all( + from_method = "from_residual", + from_desugaring = "QuestionMark", + _Self = "std::option::Option", + R = "std::result::Result", + ), + 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", @@ -272,13 +284,26 @@ pub trait Try: FromResidual { from_method = "from_residual", from_desugaring = "QuestionMark", _Self = "std::ops::ControlFlow", + R = "std::ops::ControlFlow", ), - message = "the `?` operator can only be used on `ControlFlow`s \ - in {ItemContext} that returns `ControlFlow`", + message = "the `?` operator in {ItemContext} that returns `ControlFlow` \ + can only be used on other `ControlFlow`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`" ), + on( + all( + from_method = "from_residual", + from_desugaring = "QuestionMark", + _Self = "std::ops::ControlFlow", + // `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", 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 { fn result_to_option() -> Option { 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 { @@ -30,18 +30,18 @@ fn control_flow_to_option() -> Option { fn result_to_control_flow() -> ControlFlow { ControlFlow::Continue(Err("hello")?) - //~^ ERROR the `?` operator can only be used on `ControlFlow`s in a function that returns `ControlFlow` + //~^ ERROR the `?` operator can only be used on `ControlFlow`s in a function that returns `ControlFlow` } fn option_to_control_flow() -> ControlFlow { Some(3)?; - //~^ ERROR the `?` operator can only be used on `ControlFlow`s in a function that returns `ControlFlow` + //~^ 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 { ControlFlow::Break(4_u8)?; - //~^ ERROR the `?` operator can only be used on `ControlFlow`s in a function that returns `ControlFlow` + //~^ ERROR the `?` operator in a function that returns `ControlFlow` can only be used on other `ControlFlow`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>` 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/bad-interconversion.rs:22:22 | LL | / fn result_to_option() -> Option { LL | | Some(Err("hello")?) - | | ^ this `?` produces `Result`, which is incompatible with `Option` + | | ^ use `.ok()?` if you want to discard the `Result` error information LL | | LL | | } | |_- this function returns an `Option` @@ -66,7 +66,7 @@ LL | | } = help: the trait `FromResidual>` is not implemented for `Option` = note: required by `from_residual` -error[E0277]: the `?` operator can only be used on `ControlFlow`s in a function that returns `ControlFlow` +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 { @@ -77,10 +77,9 @@ LL | | } | |_- this function returns a `ControlFlow` | = help: the trait `FromResidual>` is not implemented for `ControlFlow` - = 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`s in a function that returns `ControlFlow` +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 { @@ -92,10 +91,9 @@ LL | | } | |_- this function returns a `ControlFlow` | = help: the trait `FromResidual>` is not implemented for `ControlFlow` - = 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`s in a function that returns `ControlFlow` +error[E0277]: the `?` operator in a function that returns `ControlFlow` can only be used on other `ControlFlow`s (with the same Break type) --> $DIR/bad-interconversion.rs:43:29 | LL | / fn control_flow_to_control_flow() -> ControlFlow { 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>` 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{ LL | | let a:Result = Ok(5); LL | | a?; - | | ^ this `?` produces `Result`, which is incompatible with `Option` + | | ^ use `.ok()?` if you want to discard the `Result` error information LL | | Some(5) LL | | } | |_- this function returns an `Option`