Skip to content

Commit e3272b3

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 1280928 commit e3272b3

File tree

12 files changed

+236
-47
lines changed

12 files changed

+236
-47
lines changed

‎compiler/rustc_hir_typeck/src/coercion.rs

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -92,14 +92,17 @@ 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+
// `return` in closures does not return from the outer function
104+
hir::ExprKind::Closure(_) => return,
105+
_ => {}
103106
}
104107
intravisit::walk_expr(self, expr);
105108
}
@@ -1856,13 +1859,31 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
18561859
}
18571860

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

1861-
if let (Some(expr), Some(_), Some((fn_id, fn_decl, _, _))) =
1862-
(expression, blk_id, fcx.get_node_fn_decl(parent_item))
1863-
{
1878+
if let (Some(expr), Some(_), Some(fn_decl)) = (expression, blk_id, parent_item.fn_decl()) {
18641879
fcx.suggest_missing_break_or_return_expr(
1865-
&mut err, expr, fn_decl, expected, found, id, fn_id,
1880+
&mut err,
1881+
expr,
1882+
fn_decl,
1883+
expected,
1884+
found,
1885+
id,
1886+
parent_id.into(),
18661887
);
18671888
}
18681889

‎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: 75 additions & 24 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,
@@ -828,6 +830,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
828830
}
829831
hir::FnRetTy::Return(hir_ty) => {
830832
if let hir::TyKind::OpaqueDef(item_id, ..) = hir_ty.kind
833+
// FIXME: account for RPITIT.
831834
&& let hir::Node::Item(hir::Item {
832835
kind: hir::ItemKind::OpaqueTy(op_ty), ..
833836
}) = self.tcx.hir_node(item_id.hir_id())
@@ -1039,33 +1042,81 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10391042
return;
10401043
}
10411044

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ impl<'hir> Map<'hir> {
629629
Node::Item(_)
630630
| Node::ForeignItem(_)
631631
| Node::TraitItem(_)
632-
| Node::Expr(Expr { kind: ExprKind::Closure { .. }, .. })
632+
| Node::Expr(Expr { kind: ExprKind::Closure(_), .. })
633633
| Node::ImplItem(_)
634634
// The input node `id` must be enclosed in the method's body as opposed
635635
// 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)