Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c7d8c65
docs: clarify explicitly freeing heap allocated memory
0xalpharush Nov 4, 2023
efaf425
Add Variant and a few more APIs to stable_mir
celinval Dec 1, 2023
e19c7cd
Finish implementing `RustcInternal` for `TyKind`
celinval Dec 1, 2023
1720b10
Add FieldDef to StableMIR and methods to get type
celinval Dec 5, 2023
1a7b610
Add riscv32 imafc bare metal target
MabezDev Nov 13, 2023
68ea621
add comment about keeping flags in sync between bootstrap.py and boot…
RalfJung Dec 5, 2023
326fea0
Change ty_with_args to return Ty instead of Result
celinval Dec 5, 2023
6c3879d
Provide context when `?` can't be called because of `Result<_, E>`
estebank Oct 6, 2023
5381796
Point at fewer methods in the chain, only those that change the E type
estebank Oct 7, 2023
98e5317
Detect incorrect `;` in `Option::ok_or_else` and `Result::map_err`
estebank Oct 7, 2023
70fe624
Reduce verbosity of error
estebank Oct 7, 2023
dc0f7a1
docs: remove #110800 from release notes
notriddle Dec 6, 2023
7ff9648
library: fix comment about const assert in win api
klensy Dec 6, 2023
f1e1804
Rollup merge of #116496 - estebank:question-method-chain-context, r=c…
matthiaskrgr Dec 6, 2023
49d6594
Rollup merge of #117563 - 0xalpharush:docs/into-raw, r=workingjubilee
matthiaskrgr Dec 6, 2023
df6bc93
Rollup merge of #117874 - esp-rs:riscv3264imafc-unknown-none-elf, r=d…
matthiaskrgr Dec 6, 2023
67d8999
Rollup merge of #118516 - celinval:smir-variants, r=ouz-a
matthiaskrgr Dec 6, 2023
756bc44
Rollup merge of #118650 - RalfJung:flags-sync, r=clubby789
matthiaskrgr Dec 6, 2023
95efc0d
Rollup merge of #118664 - notriddle:master, r=Mark-Simulacrum
matthiaskrgr Dec 6, 2023
2f71a44
Rollup merge of #118669 - klensy:comment-fix, r=workingjubilee
matthiaskrgr Dec 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
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: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 Dec 5, 2023
commit 6c3879d1f154bb6f18562d29aa30fbc03239c66f
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 @@ -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,
);

fn report_const_param_not_wf(
&self,
ty: Ty<'tcx>,
Expand Down Expand Up @@ -509,6 +517,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 @@ -982,6 +994,136 @@ 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,
) {
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,
};
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
59 changes: 59 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,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(|| { //~ 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