Skip to content

Commit

Permalink
Rollup merge of rust-lang#137245 - estebank:from-residual-note-2, r=o…
Browse files Browse the repository at this point in the history
…li-obk

Tweak E0277 when predicate comes indirectly from ?

When a `?` operation requires an `Into` conversion with additional bounds (like having a concrete error but wanting to convert to a trait object), we handle it speficically and provide the same kind of information we give other `?` related errors.

```
error[E0277]: `?` couldn't convert the error: `E: std::error::Error` is not satisfied
  --> $DIR/bad-question-mark-on-trait-object.rs:7:13
   |
LL | fn foo() -> Result<(), Box<dyn std::error::Error>> {
   |             -------------------------------------- required `E: std::error::Error` because of this
LL |     Ok(bar()?)
   |        -----^ the trait `std::error::Error` is not implemented for `E`
   |        |
   |        this has type `Result<_, E>`
   |
note: `E` needs to implement `std::error::Error`
  --> $DIR/bad-question-mark-on-trait-object.rs:1:1
   |
LL | struct E;
   | ^^^^^^^^
   = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
   = note: required for `Box<dyn std::error::Error>` to implement `From<E>`
```

Avoid talking about `FromResidual` when other more relevant information is being given, particularly from `rust_on_unimplemented`.

Fix rust-lang#137238.

-----

CC rust-lang#137232, which was a smaller step related to this.
  • Loading branch information
matthiaskrgr authored Feb 22, 2025
2 parents 5400270 + 31febc6 commit 890c4d2
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -192,19 +192,38 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {

let have_alt_message = message.is_some() || label.is_some();
let is_try_conversion = self.is_try_conversion(span, main_trait_predicate.def_id());
let is_question_mark = matches!(
root_obligation.cause.code().peel_derives(),
ObligationCauseCode::QuestionMark,
) && !(
self.tcx.is_diagnostic_item(sym::FromResidual, main_trait_predicate.def_id())
|| self.tcx.is_lang_item(main_trait_predicate.def_id(), LangItem::Try)
);
let is_unsize =
self.tcx.is_lang_item(leaf_trait_predicate.def_id(), LangItem::Unsize);
let question_mark_message = "the question mark operation (`?`) implicitly \
performs a conversion on the error value \
using the `From` trait";
let (message, notes, append_const_msg) = if is_try_conversion {
// We have a `-> Result<_, E1>` and `gives_E2()?`.
(
Some(format!(
"`?` couldn't convert the error to `{}`",
main_trait_predicate.skip_binder().self_ty(),
)),
vec![
"the question mark operation (`?`) implicitly performs a \
conversion on the error value using the `From` trait"
.to_owned(),
],
vec![question_mark_message.to_owned()],
Some(AppendConstMessage::Default),
)
} else if is_question_mark {
// Similar to the case above, but in this case the conversion is for a
// trait object: `-> Result<_, Box<dyn Error>` and `gives_E()?` when
// `E: Error` isn't met.
(
Some(format!(
"`?` couldn't convert the error: `{main_trait_predicate}` is \
not satisfied",
)),
vec![question_mark_message.to_owned()],
Some(AppendConstMessage::Default),
)
} else {
Expand All @@ -220,8 +239,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
&mut long_ty_file,
);

let (err_msg, safe_transmute_explanation) = if self.tcx.is_lang_item(main_trait_predicate.def_id(), LangItem::TransmuteTrait)
{
let (err_msg, safe_transmute_explanation) = if self.tcx.is_lang_item(
main_trait_predicate.def_id(),
LangItem::TransmuteTrait,
) {
// Recompute the safe transmute reason and use that for the error reporting
match self.get_safe_transmute_error_and_reason(
obligation.clone(),
Expand Down Expand Up @@ -249,18 +270,22 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
*err.long_ty_path() = long_ty_file;

let mut suggested = false;
if is_try_conversion {
if is_try_conversion || is_question_mark {
suggested = self.try_conversion_context(&obligation, main_trait_predicate, &mut err);
}

if is_try_conversion && let Some(ret_span) = self.return_type_span(&obligation) {
err.span_label(
ret_span,
format!(
"expected `{}` because of this",
main_trait_predicate.skip_binder().self_ty()
),
);
if let Some(ret_span) = self.return_type_span(&obligation) {
if is_try_conversion {
err.span_label(
ret_span,
format!(
"expected `{}` because of this",
main_trait_predicate.skip_binder().self_ty()
),
);
} else if is_question_mark {
err.span_label(ret_span, format!("required `{main_trait_predicate}` because of this"));
}
}

if tcx.is_lang_item(leaf_trait_predicate.def_id(), LangItem::Tuple) {
Expand Down Expand Up @@ -302,10 +327,14 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
// If it has a custom `#[rustc_on_unimplemented]`
// error message, let's display it as the label!
err.span_label(span, s);
if !matches!(leaf_trait_predicate.skip_binder().self_ty().kind(), ty::Param(_)) {
if !matches!(leaf_trait_predicate.skip_binder().self_ty().kind(), ty::Param(_))
// When the self type is a type param We don't need to "the trait
// `std::marker::Sized` is not implemented for `T`" as we will point
// at the type param with a label to suggest constraining it.
&& !self.tcx.is_diagnostic_item(sym::FromResidual, leaf_trait_predicate.def_id())
// Don't say "the trait `FromResidual<Option<Infallible>>` is
// not implemented for `Result<T, E>`".
{
err.help(explanation);
}
} else if let Some(custom_explanation) = safe_transmute_explanation {
Expand Down Expand Up @@ -932,16 +961,12 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
let Some(typeck) = &self.typeck_results else {
return false;
};
let Some((ObligationCauseCode::QuestionMark, Some(y))) =
obligation.cause.code().parent_with_predicate()
else {
let ObligationCauseCode::QuestionMark = obligation.cause.code().peel_derives() else {
return false;
};
if !self.tcx.is_diagnostic_item(sym::FromResidual, y.def_id()) {
return false;
}
let self_ty = trait_pred.skip_binder().self_ty();
let found_ty = trait_pred.skip_binder().trait_ref.args.get(1).and_then(|a| a.as_type());
self.note_missing_impl_for_question_mark(err, self_ty, found_ty, trait_pred);

let mut prev_ty = self.resolve_vars_if_possible(
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
Expand Down Expand Up @@ -1106,6 +1131,56 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
suggested
}

fn note_missing_impl_for_question_mark(
&self,
err: &mut Diag<'_>,
self_ty: Ty<'_>,
found_ty: Option<Ty<'_>>,
trait_pred: ty::PolyTraitPredicate<'tcx>,
) {
match (self_ty.kind(), found_ty) {
(ty::Adt(def, _), Some(ty))
if let ty::Adt(found, _) = ty.kind()
&& def.did().is_local()
&& found.did().is_local() =>
{
err.span_note(
self.tcx.def_span(def.did()),
format!("`{self_ty}` needs to implement `From<{ty}>`"),
);
err.span_note(
self.tcx.def_span(found.did()),
format!("alternatively, `{ty}` needs to implement `Into<{self_ty}>`"),
);
}
(ty::Adt(def, _), None) if def.did().is_local() => {
err.span_note(
self.tcx.def_span(def.did()),
format!(
"`{self_ty}` needs to implement `{}`",
trait_pred.skip_binder().trait_ref.print_only_trait_path(),
),
);
}
(ty::Adt(def, _), Some(ty)) if def.did().is_local() => {
err.span_note(
self.tcx.def_span(def.did()),
format!("`{self_ty}` needs to implement `From<{ty}>`"),
);
}
(_, Some(ty))
if let ty::Adt(def, _) = ty.kind()
&& def.did().is_local() =>
{
err.span_note(
self.tcx.def_span(def.did()),
format!("`{ty}` needs to implement `Into<{self_ty}>`"),
);
}
_ => {}
}
}

fn report_const_param_not_wf(
&self,
ty: Ty<'tcx>,
Expand Down Expand Up @@ -2035,6 +2110,11 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
return false;
}
if let &[cand] = &candidates[..] {
if self.tcx.is_diagnostic_item(sym::FromResidual, cand.def_id)
&& !self.tcx.features().enabled(sym::try_trait_v2)
{
return false;
}
let (desc, mention_castable) =
match (cand.self_ty().kind(), trait_pred.self_ty().skip_binder().kind()) {
(ty::FnPtr(..), ty::FnDef(..)) => {
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/async-await/issue-84841.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ LL | | test()?;
... |
LL | | }
| |_- this function should return `Result` or `Option` to accept `?`
|
= help: the trait `FromResidual<_>` is not implemented for `()`

error: aborting due to 2 previous errors

Expand Down
6 changes: 0 additions & 6 deletions tests/ui/async-await/try-on-option-in-async.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ LL | async {
LL | let x: Option<u32> = None;
LL | x?;
| ^ cannot use the `?` operator in an async block that returns `{integer}`
|
= help: the trait `FromResidual<Option<Infallible>>` is not implemented for `{integer}`

error[E0277]: the `?` operator can only be used in an async closure that returns `Result` or `Option` (or another type that implements `FromResidual`)
--> $DIR/try-on-option-in-async.rs:16:10
Expand All @@ -20,8 +18,6 @@ LL | | x?;
LL | | 22_u32
LL | | };
| |_____- this function should return `Result` or `Option` to accept `?`
|
= help: the trait `FromResidual<Option<Infallible>>` is not implemented for `u32`

error[E0277]: the `?` operator can only be used in an async function that returns `Result` or `Option` (or another type that implements `FromResidual`)
--> $DIR/try-on-option-in-async.rs:25:6
Expand All @@ -34,8 +30,6 @@ LL | | x?;
LL | | 22
LL | | }
| |_- this function should return `Result` or `Option` to accept `?`
|
= help: the trait `FromResidual<Option<Infallible>>` is not implemented for `u32`

error: aborting due to 3 previous errors

Expand Down
6 changes: 0 additions & 6 deletions tests/ui/return/return-from-residual-sugg-issue-125997.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ LL | fn test1() {
LL | let mut _file = File::create("foo.txt")?;
| ^ cannot use the `?` operator in a function that returns `()`
|
= help: the trait `FromResidual<Result<Infallible, std::io::Error>>` is not implemented for `()`
help: consider adding return type
|
LL ~ fn test1() -> Result<(), Box<dyn std::error::Error>> {
Expand All @@ -23,7 +22,6 @@ LL | fn test2() {
LL | let mut _file = File::create("foo.txt")?;
| ^ cannot use the `?` operator in a function that returns `()`
|
= help: the trait `FromResidual<Result<Infallible, std::io::Error>>` is not implemented for `()`
help: consider adding return type
|
LL ~ fn test2() -> Result<(), Box<dyn std::error::Error>> {
Expand All @@ -41,7 +39,6 @@ LL | fn test4(&self) {
LL | let mut _file = File::create("foo.txt")?;
| ^ cannot use the `?` operator in a method that returns `()`
|
= help: the trait `FromResidual<Result<Infallible, std::io::Error>>` is not implemented for `()`
help: consider adding return type
|
LL ~ fn test4(&self) -> Result<(), Box<dyn std::error::Error>> {
Expand All @@ -59,7 +56,6 @@ LL | fn test5(&self) {
LL | let mut _file = File::create("foo.txt")?;
| ^ cannot use the `?` operator in a method that returns `()`
|
= help: the trait `FromResidual<Result<Infallible, std::io::Error>>` is not implemented for `()`
help: consider adding return type
|
LL ~ fn test5(&self) -> Result<(), Box<dyn std::error::Error>> {
Expand All @@ -78,7 +74,6 @@ LL | fn main() {
LL | let mut _file = File::create("foo.txt")?;
| ^ cannot use the `?` operator in a function that returns `()`
|
= help: the trait `FromResidual<Result<Infallible, std::io::Error>>` is not implemented for `()`
help: consider adding return type
|
LL ~ fn main() -> Result<(), Box<dyn std::error::Error>> {
Expand All @@ -99,7 +94,6 @@ LL | let mut _file = File::create("foo.txt")?;
LL | mac!();
| ------ in this macro invocation
|
= help: the trait `FromResidual<Result<Infallible, std::io::Error>>` is not implemented for `()`
= note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider adding return type
|
Expand Down
12 changes: 0 additions & 12 deletions tests/ui/try-trait/bad-interconversion.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ LL | fn option_to_result() -> Result<u64, String> {
| -------------------------------------------- this function returns a `Result`
LL | Some(3)?;
| ^ use `.ok_or(...)?` to provide an error compatible with `Result<u64, String>`
|
= help: the trait `FromResidual<Option<Infallible>>` is not implemented for `Result<u64, String>`
= help: the trait `FromResidual<Result<Infallible, E>>` is implemented for `Result<T, F>`

error[E0277]: the `?` operator can only be used on `Result`s in a function that returns `Result`
--> $DIR/bad-interconversion.rs:15:31
Expand All @@ -31,9 +28,6 @@ LL | fn control_flow_to_result() -> Result<u64, String> {
| -------------------------------------------------- this function returns a `Result`
LL | Ok(ControlFlow::Break(123)?)
| ^ this `?` produces `ControlFlow<{integer}, Infallible>`, which is incompatible with `Result<u64, String>`
|
= help: the trait `FromResidual<ControlFlow<{integer}, Infallible>>` is not implemented for `Result<u64, String>`
= help: the trait `FromResidual<Result<Infallible, E>>` is implemented for `Result<T, F>`

error[E0277]: the `?` operator can only be used on `Option`s, not `Result`s, in a function that returns `Option`
--> $DIR/bad-interconversion.rs:20:22
Expand All @@ -42,9 +36,6 @@ LL | fn result_to_option() -> Option<u16> {
| ------------------------------------ this function returns an `Option`
LL | Some(Err("hello")?)
| ^ use `.ok()?` if you want to discard the `Result<Infallible, &str>` error information
|
= help: the trait `FromResidual<Result<Infallible, &str>>` is not implemented for `Option<u16>`
= help: the trait `FromResidual<Option<Infallible>>` is implemented for `Option<T>`

error[E0277]: the `?` operator can only be used on `Option`s in a function that returns `Option`
--> $DIR/bad-interconversion.rs:25:33
Expand All @@ -53,9 +44,6 @@ LL | fn control_flow_to_option() -> Option<u64> {
| ------------------------------------------ this function returns an `Option`
LL | Some(ControlFlow::Break(123)?)
| ^ this `?` produces `ControlFlow<{integer}, Infallible>`, which is incompatible with `Option<u64>`
|
= help: the trait `FromResidual<ControlFlow<{integer}, Infallible>>` is not implemented for `Option<u64>`
= help: the trait `FromResidual<Option<Infallible>>` is implemented for `Option<T>`

error[E0277]: the `?` operator can only be used on `ControlFlow`s in a function that returns `ControlFlow`
--> $DIR/bad-interconversion.rs:30:39
Expand Down
29 changes: 29 additions & 0 deletions tests/ui/try-trait/bad-question-mark-on-trait-object.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
struct E;
//~^ NOTE `E` needs to implement `std::error::Error`
//~| NOTE alternatively, `E` needs to implement `Into<X>`
struct X; //~ NOTE `X` needs to implement `From<E>`

fn foo() -> Result<(), Box<dyn std::error::Error>> { //~ NOTE required `E: std::error::Error` because of this
Ok(bar()?)
//~^ ERROR `?` couldn't convert the error: `E: std::error::Error` is not satisfied
//~| NOTE the trait `std::error::Error` is not implemented for `E`
//~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
//~| NOTE required for `Box<dyn std::error::Error>` to implement `From<E>`
//~| NOTE this has type `Result<_, E>`
//~| NOTE in this expansion
//~| NOTE in this expansion
//~| NOTE in this expansion
}
fn bat() -> Result<(), X> { //~ NOTE expected `X` because of this
Ok(bar()?)
//~^ ERROR `?` couldn't convert the error to `X`
//~| NOTE the trait `From<E>` is not implemented for `X`
//~| NOTE this can't be annotated with `?` because it has type `Result<_, E>`
//~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
//~| NOTE in this expansion
//~| NOTE in this expansion
}
fn bar() -> Result<(), E> {
Err(E)
}
fn main() {}
43 changes: 43 additions & 0 deletions tests/ui/try-trait/bad-question-mark-on-trait-object.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
error[E0277]: `?` couldn't convert the error: `E: std::error::Error` is not satisfied
--> $DIR/bad-question-mark-on-trait-object.rs:7:13
|
LL | fn foo() -> Result<(), Box<dyn std::error::Error>> {
| -------------------------------------- required `E: std::error::Error` because of this
LL | Ok(bar()?)
| -----^ the trait `std::error::Error` is not implemented for `E`
| |
| this has type `Result<_, E>`
|
note: `E` needs to implement `std::error::Error`
--> $DIR/bad-question-mark-on-trait-object.rs:1:1
|
LL | struct E;
| ^^^^^^^^
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
= note: required for `Box<dyn std::error::Error>` to implement `From<E>`

error[E0277]: `?` couldn't convert the error to `X`
--> $DIR/bad-question-mark-on-trait-object.rs:18:13
|
LL | fn bat() -> Result<(), X> {
| ------------- expected `X` because of this
LL | Ok(bar()?)
| -----^ the trait `From<E>` is not implemented for `X`
| |
| this can't be annotated with `?` because it has type `Result<_, E>`
|
note: `X` needs to implement `From<E>`
--> $DIR/bad-question-mark-on-trait-object.rs:4:1
|
LL | struct X;
| ^^^^^^^^
note: alternatively, `E` needs to implement `Into<X>`
--> $DIR/bad-question-mark-on-trait-object.rs:1:1
|
LL | struct E;
| ^^^^^^^^
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
Loading

0 comments on commit 890c4d2

Please sign in to comment.