Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest coercion of Result using ? #106583

Merged
merged 6 commits into from
Jan 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1619,6 +1619,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
}
}
}

fn note_unreachable_loop_return(
&self,
err: &mut Diagnostic,
Expand Down
53 changes: 52 additions & 1 deletion compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|| self.suggest_copied_or_cloned(err, expr, expr_ty, expected)
|| self.suggest_clone_for_ref(err, expr, expr_ty, expected)
|| self.suggest_into(err, expr, expr_ty, expected)
|| self.suggest_floating_point_literal(err, expr, expected);
|| self.suggest_floating_point_literal(err, expr, expected)
|| self.note_result_coercion(err, expr, expected, expr_ty);
if !suggested {
self.point_at_expr_source_of_inferred_type(err, expr, expr_ty, expected);
}
Expand Down Expand Up @@ -697,6 +698,56 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}

pub(crate) fn note_result_coercion(
&self,
err: &mut Diagnostic,
expr: &hir::Expr<'tcx>,
expected: Ty<'tcx>,
found: Ty<'tcx>,
) -> bool {
let ty::Adt(e, substs_e) = expected.kind() else { return false; };
let ty::Adt(f, substs_f) = found.kind() else { return false; };
if e.did() != f.did() {
return false;
}
if Some(e.did()) != self.tcx.get_diagnostic_item(sym::Result) {
return false;
}
let map = self.tcx.hir();
if let Some(hir::Node::Expr(expr)) = map.find_parent(expr.hir_id)
&& let hir::ExprKind::Ret(_) = expr.kind
{
// `return foo;`
} else if map.get_return_block(expr.hir_id).is_some() {
// Function's tail expression.
} else {
return false;
}
let e = substs_e.type_at(1);
let f = substs_f.type_at(1);
if self
.infcx
.type_implements_trait(
self.tcx.get_diagnostic_item(sym::Into).unwrap(),
[f, e],
self.param_env,
)
.must_apply_modulo_regions()
{
err.multipart_suggestion(
"use `?` to coerce and return an appropriate `Err`, and wrap the resulting value \
in `Ok` so the expression remains of type `Result`",
vec![
(expr.span.shrink_to_lo(), "Ok(".to_string()),
(expr.span.shrink_to_hi(), "?)".to_string()),
],
Applicability::MaybeIncorrect,
);
return true;
}
false
}

/// If the expected type is an enum (Issue #55250) with any variants whose
/// sole field is of the found type, suggest such variants. (Issue #42764)
fn suggest_compatible_variants(
Expand Down
24 changes: 24 additions & 0 deletions tests/ui/type/type-check/coerce-result-return-value-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
struct A;
struct B;
impl From<A> for B {
fn from(_: A) -> Self { B }
}
fn foo4(x: Result<(), A>) -> Result<(), B> {
match true {
true => x, //~ ERROR mismatched types
false => x,
}
}
fn foo5(x: Result<(), A>) -> Result<(), B> {
match true {
true => return x, //~ ERROR mismatched types
false => return x,
}
}
fn main() {
let _ = foo4(Ok(()));
let _ = foo5(Ok(()));
let _: Result<(), B> = { //~ ERROR mismatched types
Err(A);
};
}
47 changes: 47 additions & 0 deletions tests/ui/type/type-check/coerce-result-return-value-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
error[E0308]: mismatched types
--> $DIR/coerce-result-return-value-2.rs:8:17
|
LL | fn foo4(x: Result<(), A>) -> Result<(), B> {
| ------------- expected `Result<(), B>` because of return type
LL | match true {
LL | true => x,
| ^ expected struct `B`, found struct `A`
|
= note: expected enum `Result<_, B>`
found enum `Result<_, A>`
help: use `?` to coerce and return an appropriate `Err`, and wrap the resulting value in `Ok` so the expression remains of type `Result`
|
LL | true => Ok(x?),
| +++ ++

error[E0308]: mismatched types
--> $DIR/coerce-result-return-value-2.rs:14:24
|
LL | fn foo5(x: Result<(), A>) -> Result<(), B> {
| ------------- expected `Result<(), B>` because of return type
LL | match true {
LL | true => return x,
| ^ expected struct `B`, found struct `A`
|
= note: expected enum `Result<_, B>`
found enum `Result<_, A>`
help: use `?` to coerce and return an appropriate `Err`, and wrap the resulting value in `Ok` so the expression remains of type `Result`
|
LL | true => return Ok(x?),
| +++ ++

error[E0308]: mismatched types
--> $DIR/coerce-result-return-value-2.rs:21:28
|
LL | let _: Result<(), B> = {
| ____________________________^
LL | | Err(A);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case has a mistake - you meant to test Err(A) not Err(A);.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works as a negative test :)

I don't recall now if I intended for that to be the case, which is likely not.

LL | | };
| |_____^ expected enum `Result`, found `()`
|
= note: expected enum `Result<(), B>`
found unit type `()`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0308`.
24 changes: 24 additions & 0 deletions tests/ui/type/type-check/coerce-result-return-value.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run-rustfix
struct A;
struct B;
impl From<A> for B {
fn from(_: A) -> Self { B }
}
fn foo1(x: Result<(), A>) -> Result<(), B> {
Ok(x?) //~ ERROR mismatched types
}
fn foo2(x: Result<(), A>) -> Result<(), B> {
return Ok(x?); //~ ERROR mismatched types
}
fn foo3(x: Result<(), A>) -> Result<(), B> {
if true {
Ok(x?) //~ ERROR mismatched types
} else {
Ok(x?) //~ ERROR mismatched types
}
}
fn main() {
let _ = foo1(Ok(()));
let _ = foo2(Ok(()));
let _ = foo3(Ok(()));
}
24 changes: 24 additions & 0 deletions tests/ui/type/type-check/coerce-result-return-value.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run-rustfix
struct A;
struct B;
impl From<A> for B {
fn from(_: A) -> Self { B }
}
fn foo1(x: Result<(), A>) -> Result<(), B> {
x //~ ERROR mismatched types
}
fn foo2(x: Result<(), A>) -> Result<(), B> {
return x; //~ ERROR mismatched types
}
fn foo3(x: Result<(), A>) -> Result<(), B> {
if true {
x //~ ERROR mismatched types
} else {
x //~ ERROR mismatched types
}
}
fn main() {
let _ = foo1(Ok(()));
let _ = foo2(Ok(()));
let _ = foo3(Ok(()));
}
65 changes: 65 additions & 0 deletions tests/ui/type/type-check/coerce-result-return-value.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
error[E0308]: mismatched types
--> $DIR/coerce-result-return-value.rs:8:5
|
LL | fn foo1(x: Result<(), A>) -> Result<(), B> {
| ------------- expected `Result<(), B>` because of return type
LL | x
| ^ expected struct `B`, found struct `A`
|
= note: expected enum `Result<_, B>`
found enum `Result<_, A>`
help: use `?` to coerce and return an appropriate `Err`, and wrap the resulting value in `Ok` so the expression remains of type `Result`
|
LL | Ok(x?)
| +++ ++

error[E0308]: mismatched types
--> $DIR/coerce-result-return-value.rs:11:12
|
LL | fn foo2(x: Result<(), A>) -> Result<(), B> {
| ------------- expected `Result<(), B>` because of return type
LL | return x;
| ^ expected struct `B`, found struct `A`
|
= note: expected enum `Result<_, B>`
found enum `Result<_, A>`
help: use `?` to coerce and return an appropriate `Err`, and wrap the resulting value in `Ok` so the expression remains of type `Result`
|
LL | return Ok(x?);
| +++ ++

error[E0308]: mismatched types
--> $DIR/coerce-result-return-value.rs:15:9
|
LL | fn foo3(x: Result<(), A>) -> Result<(), B> {
| ------------- expected `Result<(), B>` because of return type
LL | if true {
LL | x
| ^ expected struct `B`, found struct `A`
|
= note: expected enum `Result<_, B>`
found enum `Result<_, A>`
help: use `?` to coerce and return an appropriate `Err`, and wrap the resulting value in `Ok` so the expression remains of type `Result`
|
LL | Ok(x?)
| +++ ++

error[E0308]: mismatched types
--> $DIR/coerce-result-return-value.rs:17:9
|
LL | fn foo3(x: Result<(), A>) -> Result<(), B> {
| ------------- expected `Result<(), B>` because of return type
...
LL | x
| ^ expected struct `B`, found struct `A`
|
= note: expected enum `Result<_, B>`
found enum `Result<_, A>`
help: use `?` to coerce and return an appropriate `Err`, and wrap the resulting value in `Ok` so the expression remains of type `Result`
|
LL | Ok(x?)
| +++ ++

error: aborting due to 4 previous errors

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