Skip to content

Commit

Permalink
Provide context when ? can't be called because of Result<_, E>
Browse files Browse the repository at this point in the history
When a method chain ending in `?` causes an E0277 because the
expression's `Result::Err` variant doesn't have a type that can be
converted to the `Result<_, E>` type parameter in the return type,
provide additional context of which parts of the chain can and can't
support the `?` operator.

```
error[E0277]: `?` couldn't convert the error to `String`
  --> $DIR/question-mark-result-err-mismatch.rs:28:25
   |
LL | fn bar() -> Result<(), String> {
   |             ------------------ expected `String` because of this
LL |     let x = foo();
   |             ----- this can be annotated with `?` because it has type `Result<String, String>`
LL |     let one = x
LL |         .map(|s| ())
   |          ----------- this can be annotated with `?` because it has type `Result<(), String>`
LL |         .map_err(|_| ())?;
   |          ---------------^ the trait `From<()>` is not implemented for `String`
   |          |
   |          this can't be annotated with `?` because it has type `Result<(), ()>`
   |
   = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
   = help: the following other types implement trait `From<T>`:
             <String as From<char>>
             <String as From<Box<str>>>
             <String as From<Cow<'a, str>>>
             <String as From<&str>>
             <String as From<&mut str>>
             <String as From<&String>>
   = note: required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`
```

Fix #72124.
  • Loading branch information
estebank committed Oct 6, 2023
1 parent 94bc9c7 commit bd7a587
Show file tree
Hide file tree
Showing 7 changed files with 300 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3437,7 +3437,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
let mut type_diffs = vec![];
if let ObligationCauseCode::ExprBindingObligation(def_id, _, _, idx) = parent_code
&& let Some(node_args) = typeck_results.node_args_opt(call_hir_id)
&& let where_clauses = self.tcx.predicates_of(def_id).instantiate(self.tcx, node_args)
&& let where_clauses =
self.tcx.predicates_of(def_id).instantiate(self.tcx, node_args)
&& let Some(where_pred) = where_clauses.predicates.get(*idx)
{
if let Some(where_pred) = where_pred.as_trait_clause()
Expand Down Expand Up @@ -3469,7 +3470,11 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
{
type_diffs = vec![
Sorts(ty::error::ExpectedFound {
expected: Ty::new_alias(self.tcx,ty::Projection, where_pred.skip_binder().projection_ty),
expected: Ty::new_alias(
self.tcx,
ty::Projection,
where_pred.skip_binder().projection_ty,
),
found,
}),
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use super::suggestions::{get_explanation_based_on_obligation, TypeErrCtxtExt as
use crate::errors::{ClosureFnMutLabel, ClosureFnOnceLabel, ClosureKindMismatch};
use crate::infer::error_reporting::{TyCategory, TypeAnnotationNeeded as ErrorCode};
use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use crate::infer::InferCtxtExt as _;
use crate::infer::{self, InferCtxt};
use crate::traits::error_reporting::infer_ctxt_ext::InferCtxtExt;
use crate::traits::error_reporting::{ambiguity, ambiguity::Ambiguity::*};
Expand Down Expand Up @@ -40,7 +41,7 @@ use rustc_session::config::{DumpSolverProofTree, TraitSolver};
use rustc_session::Limit;
use rustc_span::def_id::LOCAL_CRATE;
use rustc_span::symbol::sym;
use rustc_span::{ExpnKind, Span, DUMMY_SP};
use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP};
use std::borrow::Cow;
use std::fmt;
use std::iter;
Expand Down Expand Up @@ -104,6 +105,13 @@ pub trait TypeErrCtxtExt<'tcx> {
error: &SelectionError<'tcx>,
);

fn try_conversion_context(
&self,
obligation: &PredicateObligation<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
err: &mut Diagnostic,
);

fn report_const_param_not_wf(
&self,
ty: Ty<'tcx>,
Expand Down Expand Up @@ -498,6 +506,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {

let mut err = struct_span_err!(self.tcx.sess, span, E0277, "{}", err_msg);

if is_try_conversion {
self.try_conversion_context(&obligation, trait_ref.skip_binder(), &mut err);
}

if is_try_conversion && let Some(ret_span) = self.return_type_span(&obligation) {
err.span_label(
ret_span,
Expand Down Expand Up @@ -924,6 +936,134 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
err.emit();
}

/// When the `E` of the resulting `Result<T, E>` in an expression `foo().bar().baz()?`,
/// identify thoe method chain sub-expressions that could or could not have been annotated
/// with `?`.
fn try_conversion_context(
&self,
obligation: &PredicateObligation<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
err: &mut Diagnostic,
) {
let span = obligation.cause.span;
struct V<'v> {
search_span: Span,
found: Option<&'v hir::Expr<'v>>,
}
impl<'v> Visitor<'v> for V<'v> {
fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) {
if let hir::ExprKind::Match(expr, _arms, hir::MatchSource::TryDesugar(_)) = ex.kind
{
if ex.span.with_lo(ex.span.hi() - BytePos(1)).source_equal(self.search_span) {
if let hir::ExprKind::Call(_, [expr, ..]) = expr.kind {
self.found = Some(expr);
return;
}
}
}
hir::intravisit::walk_expr(self, ex);
}
}
let hir_id = self.tcx.hir().local_def_id_to_hir_id(obligation.cause.body_id);
let body_id = match self.tcx.hir().find(hir_id) {
Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, _, body_id), .. })) => {
body_id
}
_ => return,
};
let mut v = V { search_span: span, found: None };
v.visit_body(self.tcx.hir().body(*body_id));
let Some(expr) = v.found else {
return;
};
let Some(typeck) = &self.typeck_results else {
return;
};
let Some((ObligationCauseCode::QuestionMark, Some(y))) = obligation.cause.code().parent()
else {
return;
};
if !self.tcx.is_diagnostic_item(sym::FromResidual, y.def_id()) {
return;
}
let self_ty = trait_ref.self_ty();

let mut prev_ty = self.resolve_vars_if_possible(
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
);
let mut annotate_expr = |span: Span, prev_ty: Ty<'tcx>, self_ty: Ty<'tcx>| -> bool {
// We always look at the `E` type, because that's the only one affected by `?`. If the
// incorrect `Result<T, E>` is because of the `T`, we'll get an E0308 on the whole
// expression, after the `?` has "unwrapped" the `T`.
let ty::Adt(def, args) = prev_ty.kind() else {
return false;
};
let Some(arg) = args.get(1) else {
return false;
};
if !self.tcx.is_diagnostic_item(sym::Result, def.did()) {
return false;
}
let can = if self
.infcx
.type_implements_trait(
self.tcx.get_diagnostic_item(sym::From).unwrap(),
[self_ty.into(), *arg],
obligation.param_env,
)
.must_apply_modulo_regions()
{
"can"
} else {
"can't"
};
err.span_label(
span,
format!("this {can} be annotated with `?` because it has type `{prev_ty}`"),
);
true
};

// The following logic is simlar to `point_at_chain`, but that's focused on associated types
let mut expr = expr;
while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind {
// Point at every method call in the chain with the `Result` type.
// let foo = bar.iter().map(mapper)?;
// ------ -----------
expr = rcvr_expr;
if !annotate_expr(span, prev_ty, self_ty) {
break;
}

prev_ty = self.resolve_vars_if_possible(
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
);

if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind
&& let hir::Path { res: hir::def::Res::Local(hir_id), .. } = path
&& let Some(hir::Node::Pat(binding)) = self.tcx.hir().find(*hir_id)
&& let Some(parent) = self.tcx.hir().find_parent(binding.hir_id)
{
// We've reached the root of the method call chain...
if let hir::Node::Local(local) = parent && let Some(binding_expr) = local.init {
// ...and it is a binding. Get the binding creation and continue the chain.
expr = binding_expr;
}
if let hir::Node::Param(_param) = parent {
// ...and it is a an fn argument.
break;
}
}
}
// `expr` is now the "root" expression of the method call chain, which can be any
// expression kind, like a method call or a path. If this expression is `Result<T, E>` as
// well, then we also point at it.
prev_ty = self.resolve_vars_if_possible(
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
);
annotate_expr(expr.span, prev_ty, self_ty);
}

fn report_const_param_not_wf(
&self,
ty: Ty<'tcx>,
Expand Down
60 changes: 60 additions & 0 deletions tests/ui/traits/question-mark-result-err-mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
fn foo() -> Result<String, String> { //~ NOTE expected `String` because of this
let test = String::from("one,two");
let x = test
.split_whitespace()
.next()
.ok_or_else(|| { //~ NOTE this can be annotated with `?` because it has type `Result<&str, &str>`
"Couldn't split the test string"
});
let one = x
.map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), &str>`
.map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<(), ()>`
.map(|()| "")?; //~ ERROR `?` couldn't convert the error to `String`
//~^ NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE this can't be annotated with `?` because it has type `Result<&str, ()>`
//~| NOTE the trait `From<()>` is not implemented for `String`
//~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
//~| NOTE required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
Ok(one.to_string())
}

fn bar() -> Result<(), String> { //~ NOTE expected `String` because of this
let x = foo(); //~ NOTE this can be annotated with `?` because it has type `Result<String, String>`
let one = x
.map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), String>`
.map_err(|_| ())?; //~ ERROR `?` couldn't convert the error to `String`
//~^ NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE this can't be annotated with `?` because it has type `Result<(), ()>`
//~| NOTE the trait `From<()>` is not implemented for `String`
//~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
//~| NOTE required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`
Ok(one)
}

fn baz() -> Result<String, String> { //~ NOTE expected `String` because of this
let test = String::from("one,two");
let one = test
.split_whitespace()
.next()
.ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<&str, ()>`
"Couldn't split the test string";
})?;
//~^ ERROR `?` couldn't convert the error to `String`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
//~| NOTE the trait `From<()>` is not implemented for `String`
//~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
//~| NOTE required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
Ok(one.to_string())
}

fn main() {}

83 changes: 83 additions & 0 deletions tests/ui/traits/question-mark-result-err-mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
error[E0277]: `?` couldn't convert the error to `String`
--> $DIR/question-mark-result-err-mismatch.rs:12:22
|
LL | fn foo() -> Result<String, String> {
| ---------------------- expected `String` because of this
...
LL | .ok_or_else(|| {
| __________-
LL | | "Couldn't split the test string"
LL | | });
| |__________- this can be annotated with `?` because it has type `Result<&str, &str>`
LL | let one = x
LL | .map(|s| ())
| ----------- this can be annotated with `?` because it has type `Result<(), &str>`
LL | .map_err(|_| ())
| --------------- this can't be annotated with `?` because it has type `Result<(), ()>`
LL | .map(|()| "")?;
| ------------^ the trait `From<()>` is not implemented for `String`
| |
| this can't be annotated with `?` because it has type `Result<&str, ()>`
|
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
= help: the following other types implement trait `From<T>`:
<String as From<char>>
<String as From<Box<str>>>
<String as From<Cow<'a, str>>>
<String as From<&str>>
<String as From<&mut str>>
<String as From<&String>>
= note: required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`

error[E0277]: `?` couldn't convert the error to `String`
--> $DIR/question-mark-result-err-mismatch.rs:28:25
|
LL | fn bar() -> Result<(), String> {
| ------------------ expected `String` because of this
LL | let x = foo();
| ----- this can be annotated with `?` because it has type `Result<String, String>`
LL | let one = x
LL | .map(|s| ())
| ----------- this can be annotated with `?` because it has type `Result<(), String>`
LL | .map_err(|_| ())?;
| ---------------^ the trait `From<()>` is not implemented for `String`
| |
| this can't be annotated with `?` because it has type `Result<(), ()>`
|
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
= help: the following other types implement trait `From<T>`:
<String as From<char>>
<String as From<Box<str>>>
<String as From<Cow<'a, str>>>
<String as From<&str>>
<String as From<&mut str>>
<String as From<&String>>
= note: required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`

error[E0277]: `?` couldn't convert the error to `String`
--> $DIR/question-mark-result-err-mismatch.rs:47:11
|
LL | fn baz() -> Result<String, String> {
| ---------------------- expected `String` because of this
...
LL | .ok_or_else(|| {
| __________-
LL | | "Couldn't split the test string";
LL | | })?;
| | -^ the trait `From<()>` is not implemented for `String`
| |__________|
| this can't be annotated with `?` because it has type `Result<&str, ()>`
|
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
= help: the following other types implement trait `From<T>`:
<String as From<char>>
<String as From<Box<str>>>
<String as From<Cow<'a, str>>>
<String as From<&str>>
<String as From<&mut str>>
<String as From<&String>>
= note: required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0277`.
4 changes: 3 additions & 1 deletion tests/ui/try-block/try-block-bad-type.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ error[E0277]: `?` couldn't convert the error to `TryFromSliceError`
--> $DIR/try-block-bad-type.rs:7:16
|
LL | Err("")?;
| ^ the trait `From<&str>` is not implemented for `TryFromSliceError`
| -------^ the trait `From<&str>` is not implemented for `TryFromSliceError`
| |
| this can't be annotated with `?` because it has type `Result<_, &str>`
|
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
= help: the trait `From<Infallible>` is implemented for `TryFromSliceError`
Expand Down
4 changes: 3 additions & 1 deletion tests/ui/try-trait/bad-interconversion.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ error[E0277]: `?` couldn't convert the error to `u8`
LL | fn result_to_result() -> Result<u64, u8> {
| --------------- expected `u8` because of this
LL | Ok(Err(123_i32)?)
| ^ the trait `From<i32>` is not implemented for `u8`
| ------------^ the trait `From<i32>` is not implemented for `u8`
| |
| this can't be annotated with `?` because it has type `Result<_, i32>`
|
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
= help: the following other types implement trait `From<T>`:
Expand Down
4 changes: 3 additions & 1 deletion tests/ui/try-trait/issue-32709.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ error[E0277]: `?` couldn't convert the error to `()`
LL | fn a() -> Result<i32, ()> {
| --------------- expected `()` because of this
LL | Err(5)?;
| ^ the trait `From<{integer}>` is not implemented for `()`
| ------^ the trait `From<{integer}>` is not implemented for `()`
| |
| this can't be annotated with `?` because it has type `Result<_, {integer}>`
|
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
= help: the following other types implement trait `From<T>`:
Expand Down

0 comments on commit bd7a587

Please sign in to comment.