bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #116496 - estebank:question-method-chain-context, r=compiler-errors

Browse source 
Provide context when `?` can't be called because of `Result<_, E>`

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:27:25
   |
LL | fn bar() -> Result<(), String> {
   |             ------------------ expected `String` because of this
LL |     let x = foo();
   |             ----- this 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.
This commit is contained in:
Matthias Krüger 2023-12-06 17:21:56 +01:00 • committed by GitHub
commit f1e180451b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 364 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

2
compiler/rustc_span/src/symbol.rs
View file

@ -973,6 +973,7 @@ symbols! {
managed_boxes,
manually_drop,
map,
map_err,
marker,
marker_trait_attr,
masked,
@ -1137,6 +1138,7 @@ symbols! {
offset,
offset_of,
offset_of_enum,
ok_or_else,
omit_gdb_pretty_printer_section,
on,
on_unimplemented,

235
compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
View file

@ -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::*};
@ -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, Symbol, DUMMY_SP};
use std::borrow::Cow;
use std::fmt;
use std::iter;
@ -106,6 +107,13 @@ pub trait TypeErrCtxtExt<'tcx> {
fn fn_arg_obligation(&self, obligation: &PredicateObligation<'tcx>) -> bool;
fn try_conversion_context(
&self,
obligation: &PredicateObligation<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
err: &mut Diagnostic,
) -> bool;
fn report_const_param_not_wf(
&self,
ty: Ty<'tcx>,
@ -509,6 +517,11 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
let mut err = struct_span_err!(self.tcx.sess, span, E0277, "{}", err_msg);
let mut suggested = false;
if is_try_conversion {
suggested = 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,
@ -609,8 +622,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
self.suggest_floating_point_literal(&obligation, &mut err, &trait_ref);
self.suggest_dereferencing_index(&obligation, &mut err, trait_predicate);
let mut suggested =
self.suggest_dereferences(&obligation, &mut err, trait_predicate);
suggested |= self.suggest_dereferences(&obligation, &mut err, trait_predicate);
suggested |= self.suggest_fn_call(&obligation, &mut err, trait_predicate);
let impl_candidates = self.find_similar_impl_candidates(trait_predicate);
suggested = if let &[cand] = &impl_candidates[..] {
@ -982,6 +994,223 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
false
}
/// 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,
) -> bool {
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.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 false,
};
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 false;
};
let Some(typeck) = &self.typeck_results else {
return false;
};
let Some((ObligationCauseCode::QuestionMark, Some(y))) = obligation.cause.code().parent()
else {
return false;
};
if !self.tcx.is_diagnostic_item(sym::FromResidual, y.def_id()) {
return false;
}
let self_ty = trait_ref.self_ty();
let found_ty = trait_ref.args.get(1).and_then(|a| a.as_type());
let mut prev_ty = self.resolve_vars_if_possible(
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
);
// 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 get_e_type = |prev_ty: Ty<'tcx>| -> Option<Ty<'tcx>> {
let ty::Adt(def, args) = prev_ty.kind() else {
return None;
};
let Some(arg) = args.get(1) else {
return None;
};
if !self.tcx.is_diagnostic_item(sym::Result, def.did()) {
return None;
}
Some(arg.as_type()?)
};
let mut suggested = false;
let mut chain = vec![];
// 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;
chain.push((span, prev_ty));
let next_ty = self.resolve_vars_if_possible(
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
);
let is_diagnostic_item = |symbol: Symbol, ty: Ty<'tcx>| {
let ty::Adt(def, _) = ty.kind() else {
return false;
};
self.tcx.is_diagnostic_item(symbol, def.did())
};
// For each method in the chain, see if this is `Result::map_err` or
// `Option::ok_or_else` and if it is, see if the closure passed to it has an incorrect
// trailing `;`.
if let Some(ty) = get_e_type(prev_ty)
&& let Some(found_ty) = found_ty
// Ideally we would instead use `FnCtxt::lookup_method_for_diagnostic` for 100%
// accurate check, but we are in the wrong stage to do that and looking for
// `Result::map_err` by checking the Self type and the path segment is enough.
// sym::ok_or_else
&& (
( // Result::map_err
path_segment.ident.name == sym::map_err
&& is_diagnostic_item(sym::Result, next_ty)
) || ( // Option::ok_or_else
path_segment.ident.name == sym::ok_or_else
&& is_diagnostic_item(sym::Option, next_ty)
)
)
// Found `Result<_, ()>?`
&& let ty::Tuple(tys) = found_ty.kind()
&& tys.is_empty()
// The current method call returns `Result<_, ()>`
&& self.can_eq(obligation.param_env, ty, found_ty)
// There's a single argument in the method call and it is a closure
&& args.len() == 1
&& let Some(arg) = args.get(0)
&& let hir::ExprKind::Closure(closure) = arg.kind
// The closure has a block for its body with no tail expression
&& let body = self.tcx.hir().body(closure.body)
&& let hir::ExprKind::Block(block, _) = body.value.kind
&& let None = block.expr
// The last statement is of a type that can be converted to the return error type
&& let [.., stmt] = block.stmts
&& let hir::StmtKind::Semi(expr) = stmt.kind
&& let expr_ty = self.resolve_vars_if_possible(
typeck.expr_ty_adjusted_opt(expr)
.unwrap_or(Ty::new_misc_error(self.tcx)),
)
&& self
.infcx
.type_implements_trait(
self.tcx.get_diagnostic_item(sym::From).unwrap(),
[self_ty, expr_ty],
obligation.param_env,
)
.must_apply_modulo_regions()
{
suggested = true;
err.span_suggestion_short(
stmt.span.with_lo(expr.span.hi()),
"remove this semicolon",
String::new(),
Applicability::MachineApplicable,
);
}
prev_ty = next_ty;
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)),
);
chain.push((expr.span, prev_ty));
let mut prev = None;
for (span, err_ty) in chain.into_iter().rev() {
let err_ty = get_e_type(err_ty);
let err_ty = match (err_ty, prev) {
(Some(err_ty), Some(prev)) if !self.can_eq(obligation.param_env, err_ty, prev) => {
err_ty
}
(Some(err_ty), None) => err_ty,
_ => {
prev = err_ty;
continue;
}
};
if self
.infcx
.type_implements_trait(
self.tcx.get_diagnostic_item(sym::From).unwrap(),
[self_ty, err_ty],
obligation.param_env,
)
.must_apply_modulo_regions()
{
if !suggested {
err.span_label(span, format!("this has type `Result<_, {err_ty}>`"));
}
} else {
err.span_label(
span,
format!(
"this can't be annotated with `?` because it has type `Result<_, {err_ty}>`",
),
);
}
prev = Some(err_ty);
}
suggested
}
fn report_const_param_not_wf(
&self,
ty: Ty<'tcx>,

59
tests/ui/traits/question-mark-result-err-mismatch.rs Normal file
View file

@ -0,0 +1,59 @@
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(|| {
"Couldn't split the test string"
});
let one = x
.map(|s| ())
.map_err(|e| { //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>`
e; //~ HELP remove this semicolon
})
.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 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 has type `Result<_, String>`
let one = x
.map(|s| ())
.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, ()>>`
//~| HELP the following other types implement trait `From<T>`:
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<_, ()>`
"Couldn't split the test string"; //~ HELP remove this semicolon
})?;
//~^ 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 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() {}

62
tests/ui/traits/question-mark-result-err-mismatch.stderr Normal file
View file

@ -0,0 +1,62 @@
error[E0277]: `?` couldn't convert the error to `String`
--> $DIR/question-mark-result-err-mismatch.rs:14:22
|
LL | fn foo() -> Result<String, String> {
| ---------------------- expected `String` because of this
...
LL | .map_err(|e| {
| __________-
LL | | e;
| | - help: remove this semicolon
LL | | })
| |__________- this can't be annotated with `?` because it has type `Result<_, ()>`
LL | .map(|()| "")?;
| ^ 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, ()>>`
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 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:48:11
|
LL | fn baz() -> Result<String, String> {
| ---------------------- expected `String` because of this
...
LL | .ok_or_else(|| {
| __________-
LL | | "Couldn't split the test string";
| | - help: remove this semicolon
LL | | })?;
| | -^ 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
= 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
tests/ui/try-block/try-block-bad-type.stderr
View file

@ -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`

4
tests/ui/try-trait/bad-interconversion.stderr
View file

@ -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>`:

4
tests/ui/try-trait/issue-32709.stderr
View file

@ -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>`: