Skip to content

Commit 6add192

Browse files
estebankestebank
committed
Properly handle async blocks and fns in if exprs without else
When encountering a tail expression in the then arm of an `if` expression without an `else` arm, account for `async fn` and `async` blocks to suggest `return`ing the value and pointing at the return type of the `async fn`. We now also account for AFIT when looking for the return type to point at. Fix #115405.
1 parent bf3c6c5 commit 6add192

File tree

12 files changed

+237
-48
lines changed

12 files changed

+237
-48
lines changed

‎compiler/rustc_hir_typeck/src/coercion.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -92,14 +92,16 @@ impl<'a, 'tcx> Deref for Coerce<'a, 'tcx> {
9292

9393
type CoerceResult<'tcx> = InferResult<'tcx, (Vec<Adjustment<'tcx>>, Ty<'tcx>)>;
9494

95-
struct CollectRetsVisitor<'tcx> {
96-
ret_exprs: Vec<&'tcx hir::Expr<'tcx>>,
95+
pub struct CollectRetsVisitor<'tcx> {
96+
pub ret_exprs: Vec<&'tcx hir::Expr<'tcx>>,
9797
}
9898

9999
impl<'tcx> Visitor<'tcx> for CollectRetsVisitor<'tcx> {
100100
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
101-
if let hir::ExprKind::Ret(_) = expr.kind {
102-
self.ret_exprs.push(expr);
101+
match expr.kind {
102+
hir::ExprKind::Ret(_) => self.ret_exprs.push(expr),
103+
hir::ExprKind::Closure(_) => return,
104+
_ => {}
103105
}
104106
intravisit::walk_expr(self, expr);
105107
}
@@ -1856,13 +1858,31 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
18561858
}
18571859

18581860
let parent_id = fcx.tcx.hir().get_parent_item(id);
1859-
let parent_item = fcx.tcx.hir_node_by_def_id(parent_id.def_id);
1861+
let mut parent_item = fcx.tcx.hir_node_by_def_id(parent_id.def_id);
1862+
// When suggesting return, we need to account for closures and async blocks, not just items.
1863+
for (_, node) in fcx.tcx.hir().parent_iter(id) {
1864+
match node {
1865+
hir::Node::Expr(&hir::Expr {
1866+
kind: hir::ExprKind::Closure(hir::Closure { .. }),
1867+
..
1868+
}) => {
1869+
parent_item = node;
1870+
break;
1871+
}
1872+
hir::Node::Item(_) | hir::Node::TraitItem(_) | hir::Node::ImplItem(_) => break,
1873+
_ => {}
1874+
}
1875+
}
18601876

1861-
if let (Some(expr), Some(_), Some((fn_id, fn_decl, _, _))) =
1862-
(expression, blk_id, fcx.get_node_fn_decl(parent_item))
1863-
{
1877+
if let (Some(expr), Some(_), Some(fn_decl)) = (expression, blk_id, parent_item.fn_decl()) {
18641878
fcx.suggest_missing_break_or_return_expr(
1865-
&mut err, expr, fn_decl, expected, found, id, fn_id,
1879+
&mut err,
1880+
expr,
1881+
fn_decl,
1882+
expected,
1883+
found,
1884+
id,
1885+
parent_id.into(),
18661886
);
18671887
}
18681888

‎compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -955,14 +955,35 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
955955
owner_id,
956956
..
957957
}) => Some((hir::HirId::make_owner(owner_id.def_id), &sig.decl, ident, false)),
958-
Node::Expr(&hir::Expr { hir_id, kind: hir::ExprKind::Closure(..), .. })
959-
if let Some(Node::Item(&hir::Item {
960-
ident,
961-
kind: hir::ItemKind::Fn(ref sig, ..),
962-
owner_id,
963-
..
964-
})) = self.tcx.hir().find_parent(hir_id) =>
965-
{
958+
Node::Expr(&hir::Expr {
959+
hir_id,
960+
kind:
961+
hir::ExprKind::Closure(hir::Closure {
962+
kind: hir::ClosureKind::Coroutine(..), ..
963+
}),
964+
..
965+
}) => {
966+
let (ident, sig, owner_id) = match self.tcx.hir().find_parent(hir_id) {
967+
Some(Node::Item(&hir::Item {
968+
ident,
969+
kind: hir::ItemKind::Fn(ref sig, ..),
970+
owner_id,
971+
..
972+
})) => (ident, sig, owner_id),
973+
Some(Node::TraitItem(&hir::TraitItem {
974+
ident,
975+
kind: hir::TraitItemKind::Fn(ref sig, ..),
976+
owner_id,
977+
..
978+
})) => (ident, sig, owner_id),
979+
Some(Node::ImplItem(&hir::ImplItem {
980+
ident,
981+
kind: hir::ImplItemKind::Fn(ref sig, ..),
982+
owner_id,
983+
..
984+
})) => (ident, sig, owner_id),
985+
_ => return None,
986+
};
966987
Some((
967988
hir::HirId::make_owner(owner_id.def_id),
968989
&sig.decl,

‎compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1726,7 +1726,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
17261726
}
17271727

17281728
/// Given a function block's `HirId`, returns its `FnDecl` if it exists, or `None` otherwise.
1729-
fn get_parent_fn_decl(&self, blk_id: hir::HirId) -> Option<(&'tcx hir::FnDecl<'tcx>, Ident)> {
1729+
pub(crate) fn get_parent_fn_decl(
1730+
&self,
1731+
blk_id: hir::HirId,
1732+
) -> Option<(&'tcx hir::FnDecl<'tcx>, Ident)> {
17301733
let parent = self.tcx.hir_node_by_def_id(self.tcx.hir().get_parent_item(blk_id).def_id);
17311734
self.get_node_fn_decl(parent).map(|(_, fn_decl, ident, _)| (fn_decl, ident))
17321735
}

‎compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

Lines changed: 77 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use super::FnCtxt;
22

3+
use crate::coercion::CollectRetsVisitor;
34
use crate::errors;
45
use crate::fluent_generated as fluent;
56
use crate::fn_ctxt::rustc_span::BytePos;
@@ -16,6 +17,7 @@ use rustc_errors::{Applicability, Diagnostic, MultiSpan};
1617
use rustc_hir as hir;
1718
use rustc_hir::def::Res;
1819
use rustc_hir::def::{CtorKind, CtorOf, DefKind};
20+
use rustc_hir::intravisit::{Map, Visitor};
1921
use rustc_hir::lang_items::LangItem;
2022
use rustc_hir::{
2123
CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind, GenericBound, HirId, Node,
@@ -826,6 +828,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
826828
}
827829
hir::FnRetTy::Return(hir_ty) => {
828830
if let hir::TyKind::OpaqueDef(item_id, ..) = hir_ty.kind
831+
// FIXME: account for RPITIT.
829832
&& let hir::Node::Item(hir::Item {
830833
kind: hir::ItemKind::OpaqueTy(op_ty), ..
831834
}) = self.tcx.hir_node(item_id.hir_id())
@@ -1037,33 +1040,82 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10371040
return;
10381041
}
10391042

1040-
if let hir::FnRetTy::Return(ty) = fn_decl.output {
1041-
let ty = self.astconv().ast_ty_to_ty(ty);
1042-
let bound_vars = self.tcx.late_bound_vars(fn_id);
1043-
let ty = self
1044-
.tcx
1045-
.instantiate_bound_regions_with_erased(Binder::bind_with_vars(ty, bound_vars));
1046-
let ty = match self.tcx.asyncness(fn_id.owner) {
1047-
ty::Asyncness::Yes => self.get_impl_future_output_ty(ty).unwrap_or_else(|| {
1048-
span_bug!(fn_decl.output.span(), "failed to get output type of async function")
1049-
}),
1050-
ty::Asyncness::No => ty,
1051-
};
1052-
let ty = self.normalize(expr.span, ty);
1053-
if self.can_coerce(found, ty) {
1054-
if let Some(node) = self.tcx.opt_hir_node(fn_id)
1055-
&& let Some(owner_node) = node.as_owner()
1056-
&& let Some(span) = expr.span.find_ancestor_inside(owner_node.span())
1043+
let in_closure = matches!(
1044+
self.tcx
1045+
.hir()
1046+
.parent_iter(id)
1047+
.filter(|(_, node)| {
1048+
matches!(
1049+
node,
1050+
Node::Expr(Expr { kind: ExprKind::Closure(..), .. })
1051+
| Node::Item(_)
1052+
| Node::TraitItem(_)
1053+
| Node::ImplItem(_)
1054+
)
1055+
})
1056+
.next(),
1057+
Some((_, Node::Expr(Expr { kind: ExprKind::Closure(..), .. })))
1058+
);
1059+
1060+
let can_return = match fn_decl.output {
1061+
hir::FnRetTy::Return(ty) => {
1062+
let ty = self.astconv().ast_ty_to_ty(ty);
1063+
let bound_vars = self.tcx.late_bound_vars(fn_id);
1064+
let ty = self
1065+
.tcx
1066+
.instantiate_bound_regions_with_erased(Binder::bind_with_vars(ty, bound_vars));
1067+
let ty = match self.tcx.asyncness(fn_id.owner) {
1068+
ty::Asyncness::Yes => self.get_impl_future_output_ty(ty).unwrap_or_else(|| {
1069+
span_bug!(
1070+
fn_decl.output.span(),
1071+
"failed to get output type of async function"
1072+
)
1073+
}),
1074+
ty::Asyncness::No => ty,
1075+
};
1076+
let ty = self.normalize(expr.span, ty);
1077+
self.can_coerce(found, ty)
1078+
}
1079+
hir::FnRetTy::DefaultReturn(_) if in_closure => {
1080+
let mut rets = vec![];
1081+
if let Some(ret_coercion) = self.ret_coercion.as_ref() {
1082+
let ret_ty = ret_coercion.borrow().expected_ty();
1083+
rets.push(ret_ty);
1084+
}
1085+
let mut visitor = CollectRetsVisitor { ret_exprs: vec![] };
1086+
if let Some(item) = self.tcx.hir().find(id)
1087+
&& let Node::Expr(expr) = item
10571088
{
1058-
err.multipart_suggestion(
1059-
"you might have meant to return this value",
1060-
vec![
1061-
(span.shrink_to_lo(), "return ".to_string()),
1062-
(span.shrink_to_hi(), ";".to_string()),
1063-
],
1064-
Applicability::MaybeIncorrect,
1065-
);
1089+
visitor.visit_expr(expr);
1090+
for expr in visitor.ret_exprs {
1091+
if let Some(ty) = self.typeck_results.borrow().node_type_opt(expr.hir_id) {
1092+
rets.push(ty);
1093+
}
1094+
}
1095+
if let hir::ExprKind::Block(hir::Block { expr: Some(expr), .. }, _) = expr.kind
1096+
{
1097+
if let Some(ty) = self.typeck_results.borrow().node_type_opt(expr.hir_id) {
1098+
rets.push(ty);
1099+
}
1100+
}
10661101
}
1102+
rets.into_iter().all(|ty| self.can_coerce(found, ty))
1103+
}
1104+
_ => false,
1105+
};
1106+
if can_return {
1107+
if let Some(node) = self.tcx.opt_hir_node(fn_id)
1108+
&& let Some(owner_node) = node.as_owner()
1109+
&& let Some(span) = expr.span.find_ancestor_inside(owner_node.span())
1110+
{
1111+
err.multipart_suggestion(
1112+
"you might have meant to return this value",
1113+
vec![
1114+
(span.shrink_to_lo(), "return ".to_string()),
1115+
(span.shrink_to_hi(), ";".to_string()),
1116+
],
1117+
Applicability::MaybeIncorrect,
1118+
);
10671119
}
10681120
}
10691121
}

‎compiler/rustc_middle/src/hir/map/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ impl<'hir> Map<'hir> {
644644
Node::Item(_)
645645
| Node::ForeignItem(_)
646646
| Node::TraitItem(_)
647-
| Node::Expr(Expr { kind: ExprKind::Closure { .. }, .. })
647+
| Node::Expr(Expr { kind: ExprKind::Closure(_), .. })
648648
| Node::ImplItem(_)
649649
// The input node `id` must be enclosed in the method's body as opposed
650650
// to some other place such as its return type (fixes #114918).

‎compiler/rustc_parse/src/parser/diagnostics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,7 @@ impl<'a> Parser<'a> {
900900
// fn foo() -> Foo {
901901
// field: value,
902902
// }
903-
info!(?maybe_struct_name, ?self.token);
903+
debug!(?maybe_struct_name, ?self.token);
904904
let mut snapshot = self.create_snapshot_for_diagnostic();
905905
let path = Path {
906906
segments: ThinVec::new(),

‎tests/ui/async-await/missing-return-in-async-block.fixed

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// run-rustfix
2+
// edition:2021
3+
use std::future::Future;
4+
use std::pin::Pin;
5+
pub struct S;
6+
pub fn foo() {
7+
let _ = Box::pin(async move {
8+
if true {
9+
return Ok(S); //~ ERROR mismatched types
10+
}
11+
Err(())
12+
});
13+
}
14+
pub fn bar() -> Pin<Box<dyn Future<Output = Result<S, ()>> + 'static>> {
15+
Box::pin(async move {
16+
if true {
17+
return Ok(S); //~ ERROR mismatched types
18+
}
19+
Err(())
20+
})
21+
}
22+
fn main() {}

‎tests/ui/async-await/missing-return-in-async-block.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// run-rustfix
2+
// edition:2021
3+
use std::future::Future;
4+
use std::pin::Pin;
5+
pub struct S;
6+
pub fn foo() {
7+
let _ = Box::pin(async move {
8+
if true {
9+
Ok(S) //~ ERROR mismatched types
10+
}
11+
Err(())
12+
});
13+
}
14+
pub fn bar() -> Pin<Box<dyn Future<Output = Result<S, ()>> + 'static>> {
15+
Box::pin(async move {
16+
if true {
17+
Ok(S) //~ ERROR mismatched types
18+
}
19+
Err(())
20+
})
21+
}
22+
fn main() {}

‎tests/ui/async-await/missing-return-in-async-block.stderr

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/missing-return-in-async-block.rs:9:13
3+
|
4+
LL | / if true {
5+
LL | | Ok(S)
6+
| | ^^^^^ expected `()`, found `Result<S, _>`
7+
LL | | }
8+
| |_________- expected this to be `()`
9+
|
10+
= note: expected unit type `()`
11+
found enum `Result<S, _>`
12+
help: you might have meant to return this value
13+
|
14+
LL | return Ok(S);
15+
| ++++++ +
16+
17+
error[E0308]: mismatched types
18+
--> $DIR/missing-return-in-async-block.rs:17:13
19+
|
20+
LL | / if true {
21+
LL | | Ok(S)
22+
| | ^^^^^ expected `()`, found `Result<S, _>`
23+
LL | | }
24+
| |_________- expected this to be `()`
25+
|
26+
= note: expected unit type `()`
27+
found enum `Result<S, _>`
28+
help: you might have meant to return this value
29+
|
30+
LL | return Ok(S);
31+
| ++++++ +
32+
33+
error: aborting due to 2 previous errors
34+
35+
For more information about this error, try `rustc --explain E0308`.

‎tests/ui/impl-trait/in-trait/default-body-type-err-2.stderr

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
error[E0308]: mismatched types
22
--> $DIR/default-body-type-err-2.rs:7:9
33
|
4+
LL | async fn woopsie_async(&self) -> String {
5+
| ------ expected `String` because of return type
46
LL | 42
57
| ^^- help: try using a conversion method: `.to_string()`
68
| |

‎tests/ui/loops/dont-suggest-break-thru-item.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ fn closure() {
88
if true {
99
Err(1)
1010
//~^ ERROR mismatched types
11+
//~| HELP you might have meant to return this value
1112
}
1213

1314
Ok(())
@@ -21,6 +22,7 @@ fn async_block() {
2122
if true {
2223
Err(1)
2324
//~^ ERROR mismatched types
25+
//~| HELP you might have meant to return this value
2426
}
2527

2628
Ok(())

0 commit comments

Comments
 (0)