Skip to content

Commit a2a6fe7

Browse files
Inline get_node_fn_decl into get_fn_decl, simplify/explain logic in report_return_mismatched_types
1 parent b826eb2 commit a2a6fe7

File tree

10 files changed

+123
-120
lines changed

10 files changed

+123
-120
lines changed

compiler/rustc_hir_typeck/src/coercion.rs

+10-23
Original file line numberDiff line numberDiff line change
@@ -1861,39 +1861,26 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
18611861
}
18621862
};
18631863

1864+
// If this is due to an explicit `return`, suggest adding a return type.
18641865
if let Some((fn_id, fn_decl, can_suggest)) = fcx.get_fn_decl(parent_id)
18651866
&& !due_to_block
18661867
{
18671868
fcx.suggest_missing_return_type(&mut err, fn_decl, expected, found, can_suggest, fn_id);
18681869
}
18691870

1870-
let mut parent_id = fcx.tcx.hir().get_parent_item(block_or_return_id).def_id;
1871-
let mut parent_item = fcx.tcx.hir_node_by_def_id(parent_id);
1872-
// When suggesting return, we need to account for closures and async blocks, not just items.
1873-
// FIXME: fix get_fn_decl to be async block aware, use get_fn_decl results above
1874-
for (_, node) in fcx.tcx.hir().parent_iter(block_or_return_id) {
1875-
match node {
1876-
hir::Node::Expr(&hir::Expr {
1877-
kind: hir::ExprKind::Closure(hir::Closure { def_id, .. }),
1878-
..
1879-
}) => {
1880-
parent_item = node;
1881-
parent_id = *def_id;
1882-
break;
1883-
}
1884-
hir::Node::Item(_) | hir::Node::TraitItem(_) | hir::Node::ImplItem(_) => break,
1885-
_ => {}
1886-
}
1887-
}
1888-
1889-
if let Some(expr) = expression
1890-
&& let Some(fn_decl) = parent_item.fn_decl()
1891-
&& due_to_block
1871+
// If this is due to a block, then maybe we forgot a `return`/`break`.
1872+
if due_to_block
1873+
&& let Some(expr) = expression
1874+
&& let Some((parent_fn_decl, parent_id)) = fcx
1875+
.tcx
1876+
.hir()
1877+
.parent_iter(block_or_return_id)
1878+
.find_map(|(_, node)| Some((node.fn_decl()?, node.associated_body()?.0)))
18921879
{
18931880
fcx.suggest_missing_break_or_return_expr(
18941881
&mut err,
18951882
expr,
1896-
fn_decl,
1883+
parent_fn_decl,
18971884
expected,
18981885
found,
18991886
block_or_return_id,

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

+68-75
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use rustc_middle::{bug, span_bug};
3333
use rustc_session::lint;
3434
use rustc_span::def_id::LocalDefId;
3535
use rustc_span::hygiene::DesugaringKind;
36-
use rustc_span::symbol::{kw, sym, Ident};
36+
use rustc_span::symbol::{kw, sym};
3737
use rustc_span::Span;
3838
use rustc_target::abi::FieldIdx;
3939
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
@@ -866,76 +866,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
866866
)
867867
}
868868

869-
/// Given a function `Node`, return its `HirId` and `FnDecl` if it exists. Given a closure
870-
/// that is the child of a function, return that function's `HirId` and `FnDecl` instead.
871-
/// This may seem confusing at first, but this is used in diagnostics for `async fn`,
872-
/// for example, where most of the type checking actually happens within a nested closure,
873-
/// but we often want access to the parent function's signature.
874-
///
875-
/// Otherwise, return false.
876-
pub(crate) fn get_node_fn_decl(
877-
&self,
878-
node: Node<'tcx>,
879-
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, Ident, bool)> {
880-
match node {
881-
Node::Item(&hir::Item {
882-
ident,
883-
kind: hir::ItemKind::Fn(ref sig, ..),
884-
owner_id,
885-
..
886-
}) => {
887-
// This is less than ideal, it will not suggest a return type span on any
888-
// method called `main`, regardless of whether it is actually the entry point,
889-
// but it will still present it as the reason for the expected type.
890-
Some((owner_id.def_id, &sig.decl, ident, ident.name != sym::main))
891-
}
892-
Node::TraitItem(&hir::TraitItem {
893-
ident,
894-
kind: hir::TraitItemKind::Fn(ref sig, ..),
895-
owner_id,
896-
..
897-
}) => Some((owner_id.def_id, &sig.decl, ident, true)),
898-
Node::ImplItem(&hir::ImplItem {
899-
ident,
900-
kind: hir::ImplItemKind::Fn(ref sig, ..),
901-
owner_id,
902-
..
903-
}) => Some((owner_id.def_id, &sig.decl, ident, false)),
904-
Node::Expr(&hir::Expr {
905-
hir_id,
906-
kind:
907-
hir::ExprKind::Closure(hir::Closure {
908-
kind: hir::ClosureKind::Coroutine(..), ..
909-
}),
910-
..
911-
}) => {
912-
let (ident, sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
913-
Node::Item(&hir::Item {
914-
ident,
915-
kind: hir::ItemKind::Fn(ref sig, ..),
916-
owner_id,
917-
..
918-
}) => (ident, sig, owner_id),
919-
Node::TraitItem(&hir::TraitItem {
920-
ident,
921-
kind: hir::TraitItemKind::Fn(ref sig, ..),
922-
owner_id,
923-
..
924-
}) => (ident, sig, owner_id),
925-
Node::ImplItem(&hir::ImplItem {
926-
ident,
927-
kind: hir::ImplItemKind::Fn(ref sig, ..),
928-
owner_id,
929-
..
930-
}) => (ident, sig, owner_id),
931-
_ => return None,
932-
};
933-
Some((owner_id.def_id, &sig.decl, ident, ident.name != sym::main))
934-
}
935-
_ => None,
936-
}
937-
}
938-
939869
/// Given a `HirId`, return the `HirId` of the enclosing function, its `FnDecl`, and whether a
940870
/// suggestion can be made, `None` otherwise.
941871
pub fn get_fn_decl(
@@ -944,10 +874,73 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
944874
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, bool)> {
945875
// Get enclosing Fn, if it is a function or a trait method, unless there's a `loop` or
946876
// `while` before reaching it, as block tail returns are not available in them.
947-
self.tcx.hir().get_fn_id_for_return_block(blk_id).and_then(|blk_id| {
948-
let parent = self.tcx.hir_node(blk_id);
949-
self.get_node_fn_decl(parent)
950-
.map(|(fn_id, fn_decl, _, is_main)| (fn_id, fn_decl, is_main))
877+
self.tcx.hir().get_fn_id_for_return_block(blk_id).and_then(|item_id| {
878+
match self.tcx.hir_node(item_id) {
879+
Node::Item(&hir::Item {
880+
ident,
881+
kind: hir::ItemKind::Fn(ref sig, ..),
882+
owner_id,
883+
..
884+
}) => {
885+
// This is less than ideal, it will not suggest a return type span on any
886+
// method called `main`, regardless of whether it is actually the entry point,
887+
// but it will still present it as the reason for the expected type.
888+
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
889+
}
890+
Node::TraitItem(&hir::TraitItem {
891+
kind: hir::TraitItemKind::Fn(ref sig, ..),
892+
owner_id,
893+
..
894+
}) => Some((owner_id.def_id, sig.decl, true)),
895+
// FIXME: Suggestable if this is not a trait implementation
896+
Node::ImplItem(&hir::ImplItem {
897+
kind: hir::ImplItemKind::Fn(ref sig, ..),
898+
owner_id,
899+
..
900+
}) => Some((owner_id.def_id, sig.decl, false)),
901+
Node::Expr(&hir::Expr {
902+
hir_id,
903+
kind: hir::ExprKind::Closure(&hir::Closure { def_id, kind, fn_decl, .. }),
904+
..
905+
}) => {
906+
match kind {
907+
hir::ClosureKind::CoroutineClosure(_) => {
908+
// FIXME(async_closures): Implement this.
909+
return None;
910+
}
911+
hir::ClosureKind::Closure => Some((def_id, fn_decl, true)),
912+
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
913+
_,
914+
hir::CoroutineSource::Fn,
915+
)) => {
916+
let (ident, sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
917+
Node::Item(&hir::Item {
918+
ident,
919+
kind: hir::ItemKind::Fn(ref sig, ..),
920+
owner_id,
921+
..
922+
}) => (ident, sig, owner_id),
923+
Node::TraitItem(&hir::TraitItem {
924+
ident,
925+
kind: hir::TraitItemKind::Fn(ref sig, ..),
926+
owner_id,
927+
..
928+
}) => (ident, sig, owner_id),
929+
Node::ImplItem(&hir::ImplItem {
930+
ident,
931+
kind: hir::ImplItemKind::Fn(ref sig, ..),
932+
owner_id,
933+
..
934+
}) => (ident, sig, owner_id),
935+
_ => return None,
936+
};
937+
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
938+
}
939+
_ => None,
940+
}
941+
}
942+
_ => None,
943+
}
951944
})
952945
}
953946

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

+7
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
800800
can_suggest: bool,
801801
fn_id: LocalDefId,
802802
) -> bool {
803+
// Can't suggest `->` on a block-like coroutine
804+
if let Some(hir::CoroutineKind::Desugared(_, hir::CoroutineSource::Block)) =
805+
self.tcx.coroutine_kind(fn_id)
806+
{
807+
return false;
808+
}
809+
803810
let found =
804811
self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(found));
805812
// Only suggest changing the return type for methods that

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,11 @@ impl<'hir> Map<'hir> {
549549
Node::Block(Block { expr: None, .. }) => return None,
550550
// The current node is not the tail expression of its parent.
551551
Node::Block(Block { expr: Some(e), .. }) if hir_id != e.hir_id => return None,
552-
Node::Block(Block { expr: Some(e), ..}) if matches!(e.kind, ExprKind::If(_, _, None)) => return None,
552+
Node::Block(Block { expr: Some(e), .. })
553+
if matches!(e.kind, ExprKind::If(_, _, None)) =>
554+
{
555+
return None;
556+
}
553557
_ => {}
554558
}
555559
}

tests/ui/async-await/dont-ice-for-type-mismatch-in-closure-in-async.stderr

+3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ LL | | }
1111
error[E0308]: mismatched types
1212
--> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:12:9
1313
|
14+
LL | call(|| -> Option<()> {
15+
| ---------- expected `Option<()>` because of return type
16+
...
1417
LL | true
1518
| ^^^^ expected `Option<()>`, found `bool`
1619
|

tests/ui/closures/closure-return-type-mismatch.stderr

+3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ LL | return "test";
1313
error[E0308]: mismatched types
1414
--> $DIR/closure-return-type-mismatch.rs:12:20
1515
|
16+
LL | || -> bool {
17+
| ---- expected `bool` because of return type
18+
LL | if false {
1619
LL | return "hello"
1720
| ^^^^^^^ expected `bool`, found `&str`
1821

tests/ui/impl-trait/issue-99914.stderr

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ error[E0308]: mismatched types
22
--> $DIR/issue-99914.rs:9:27
33
|
44
LL | t.and_then(|t| -> _ { bar(t) });
5-
| ^^^^^^ expected `Result<_, Error>`, found future
5+
| - ^^^^^^ expected `Result<_, Error>`, found future
6+
| |
7+
| expected `Result<_, Error>` because of return type
68
|
79
help: try wrapping the expression in `Ok`
810
|

tests/ui/suggestions/try-operator-dont-suggest-semicolon.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
fn main() -> Result<(), ()> {
55
a(|| {
6+
//~^ HELP: try adding a return type
67
b()
78
//~^ ERROR: mismatched types [E0308]
89
//~| NOTE: expected `()`, found `i32`

tests/ui/suggestions/try-operator-dont-suggest-semicolon.stderr

+12-5
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
error[E0308]: mismatched types
2-
--> $DIR/try-operator-dont-suggest-semicolon.rs:6:9
2+
--> $DIR/try-operator-dont-suggest-semicolon.rs:7:9
33
|
44
LL | b()
5-
| ^^^- help: consider using a semicolon here: `;`
6-
| |
7-
| expected `()`, found `i32`
5+
| ^^^ expected `()`, found `i32`
6+
|
7+
help: consider using a semicolon here
8+
|
9+
LL | b();
10+
| +
11+
help: try adding a return type
12+
|
13+
LL | a(|| -> i32 {
14+
| ++++++
815

916
error[E0308]: mismatched types
10-
--> $DIR/try-operator-dont-suggest-semicolon.rs:16:9
17+
--> $DIR/try-operator-dont-suggest-semicolon.rs:17:9
1118
|
1219
LL | / if true {
1320
LL | |

tests/ui/typeck/issue-81943.stderr

+11-15
Original file line numberDiff line numberDiff line change
@@ -2,36 +2,32 @@ error[E0308]: mismatched types
22
--> $DIR/issue-81943.rs:7:9
33
|
44
LL | f(|x| lib::d!(x));
5-
| ^^^^^^^^^^ expected `()`, found `i32`
5+
| -^^^^^^^^^^ expected `()`, found `i32`
6+
| |
7+
| help: try adding a return type: `-> i32`
68
|
79
= note: this error originates in the macro `lib::d` (in Nightly builds, run with -Z macro-backtrace for more info)
810

911
error[E0308]: mismatched types
1012
--> $DIR/issue-81943.rs:8:28
1113
|
1214
LL | f(|x| match x { tmp => { g(tmp) } });
13-
| -------------------^^^^^^----
14-
| | |
15-
| | expected `()`, found `i32`
16-
| expected this to be `()`
15+
| ^^^^^^ expected `()`, found `i32`
1716
|
1817
help: consider using a semicolon here
1918
|
2019
LL | f(|x| match x { tmp => { g(tmp); } });
2120
| +
22-
help: consider using a semicolon here
21+
help: try adding a return type
2322
|
24-
LL | f(|x| match x { tmp => { g(tmp) } };);
25-
| +
23+
LL | f(|x| -> i32 match x { tmp => { g(tmp) } });
24+
| ++++++
2625

2726
error[E0308]: mismatched types
2827
--> $DIR/issue-81943.rs:10:38
2928
|
3029
LL | ($e:expr) => { match $e { x => { g(x) } } }
31-
| ------------------^^^^----
32-
| | |
33-
| | expected `()`, found `i32`
34-
| expected this to be `()`
30+
| ^^^^ expected `()`, found `i32`
3531
LL | }
3632
LL | f(|x| d!(x));
3733
| ----- in this macro invocation
@@ -41,10 +37,10 @@ help: consider using a semicolon here
4137
|
4238
LL | ($e:expr) => { match $e { x => { g(x); } } }
4339
| +
44-
help: consider using a semicolon here
40+
help: try adding a return type
4541
|
46-
LL | ($e:expr) => { match $e { x => { g(x) } }; }
47-
| +
42+
LL | f(|x| -> i32 d!(x));
43+
| ++++++
4844

4945
error: aborting due to 3 previous errors
5046

0 commit comments

Comments
 (0)