Skip to content

Commit 9b7739c

Browse files
wafarmwafarm
committed
Don't suggest adding return type for closures with default return type
1 parent 79611d9 commit 9b7739c

11 files changed

+74
-72
lines changed

Diff for: ‎compiler/rustc_hir_typeck/src/_match.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
388388
{
389389
// check that the `if` expr without `else` is the fn body's expr
390390
if expr.span == sp {
391-
return self.get_fn_decl(hir_id).map(|(_, fn_decl, _)| {
391+
return self.get_fn_decl(hir_id).map(|(_, fn_decl)| {
392392
let (ty, span) = match fn_decl.output {
393393
hir::FnRetTy::DefaultReturn(span) => ("()".to_string(), span),
394394
hir::FnRetTy::Return(ty) => (ty_to_string(&self.tcx, ty), ty.span),

Diff for: ‎compiler/rustc_hir_typeck/src/coercion.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1859,10 +1859,10 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
18591859
};
18601860

18611861
// If this is due to an explicit `return`, suggest adding a return type.
1862-
if let Some((fn_id, fn_decl, can_suggest)) = fcx.get_fn_decl(block_or_return_id)
1862+
if let Some((fn_id, fn_decl)) = fcx.get_fn_decl(block_or_return_id)
18631863
&& !due_to_block
18641864
{
1865-
fcx.suggest_missing_return_type(&mut err, fn_decl, expected, found, can_suggest, fn_id);
1865+
fcx.suggest_missing_return_type(&mut err, fn_decl, expected, found, fn_id);
18661866
}
18671867

18681868
// If this is due to a block, then maybe we forgot a `return`/`break`.

Diff for: ‎compiler/rustc_hir_typeck/src/expr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
773773
self.ret_coercion_span.set(Some(expr.span));
774774
}
775775
let cause = self.cause(expr.span, ObligationCauseCode::ReturnNoExpression);
776-
if let Some((_, fn_decl, _)) = self.get_fn_decl(expr.hir_id) {
776+
if let Some((_, fn_decl)) = self.get_fn_decl(expr.hir_id) {
777777
coercion.coerce_forced_unit(
778778
self,
779779
&cause,

Diff for: ‎compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

+13-29
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use rustc_middle::{bug, span_bug};
3131
use rustc_session::lint;
3232
use rustc_span::def_id::LocalDefId;
3333
use rustc_span::hygiene::DesugaringKind;
34-
use rustc_span::symbol::{kw, sym};
34+
use rustc_span::symbol::kw;
3535
use rustc_span::Span;
3636
use rustc_target::abi::FieldIdx;
3737
use rustc_trait_selection::error_reporting::infer::need_type_info::TypeAnnotationNeeded;
@@ -895,38 +895,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
895895
)
896896
}
897897

898-
/// Given a `HirId`, return the `HirId` of the enclosing function, its `FnDecl`, and whether a
899-
/// suggestion can be made, `None` otherwise.
900-
pub fn get_fn_decl(
901-
&self,
902-
blk_id: HirId,
903-
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, bool)> {
898+
/// Given a `HirId`, return the `HirId` of the enclosing function and its `FnDecl`.
899+
pub fn get_fn_decl(&self, blk_id: HirId) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>)> {
904900
// Get enclosing Fn, if it is a function or a trait method, unless there's a `loop` or
905901
// `while` before reaching it, as block tail returns are not available in them.
906902
self.tcx.hir().get_fn_id_for_return_block(blk_id).and_then(|item_id| {
907903
match self.tcx.hir_node(item_id) {
908904
Node::Item(&hir::Item {
909-
ident,
910-
kind: hir::ItemKind::Fn(ref sig, ..),
911-
owner_id,
912-
..
913-
}) => {
914-
// This is less than ideal, it will not suggest a return type span on any
915-
// method called `main`, regardless of whether it is actually the entry point,
916-
// but it will still present it as the reason for the expected type.
917-
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
918-
}
905+
kind: hir::ItemKind::Fn(ref sig, ..), owner_id, ..
906+
}) => Some((owner_id.def_id, sig.decl)),
919907
Node::TraitItem(&hir::TraitItem {
920908
kind: hir::TraitItemKind::Fn(ref sig, ..),
921909
owner_id,
922910
..
923-
}) => Some((owner_id.def_id, sig.decl, true)),
924-
// FIXME: Suggestable if this is not a trait implementation
911+
}) => Some((owner_id.def_id, sig.decl)),
925912
Node::ImplItem(&hir::ImplItem {
926913
kind: hir::ImplItemKind::Fn(ref sig, ..),
927914
owner_id,
928915
..
929-
}) => Some((owner_id.def_id, sig.decl, false)),
916+
}) => Some((owner_id.def_id, sig.decl)),
930917
Node::Expr(&hir::Expr {
931918
hir_id,
932919
kind: hir::ExprKind::Closure(&hir::Closure { def_id, kind, fn_decl, .. }),
@@ -937,33 +924,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
937924
// FIXME(async_closures): Implement this.
938925
return None;
939926
}
940-
hir::ClosureKind::Closure => Some((def_id, fn_decl, true)),
927+
hir::ClosureKind::Closure => Some((def_id, fn_decl)),
941928
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
942929
_,
943930
hir::CoroutineSource::Fn,
944931
)) => {
945-
let (ident, sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
932+
let (sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
946933
Node::Item(&hir::Item {
947-
ident,
948934
kind: hir::ItemKind::Fn(ref sig, ..),
949935
owner_id,
950936
..
951-
}) => (ident, sig, owner_id),
937+
}) => (sig, owner_id),
952938
Node::TraitItem(&hir::TraitItem {
953-
ident,
954939
kind: hir::TraitItemKind::Fn(ref sig, ..),
955940
owner_id,
956941
..
957-
}) => (ident, sig, owner_id),
942+
}) => (sig, owner_id),
958943
Node::ImplItem(&hir::ImplItem {
959-
ident,
960944
kind: hir::ImplItemKind::Fn(ref sig, ..),
961945
owner_id,
962946
..
963-
}) => (ident, sig, owner_id),
947+
}) => (sig, owner_id),
964948
_ => return None,
965949
};
966-
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
950+
Some((owner_id.def_id, sig.decl))
967951
}
968952
_ => None,
969953
}

Diff for: ‎compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1841,7 +1841,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
18411841
// that highlight errors inline.
18421842
let mut sp = blk.span;
18431843
let mut fn_span = None;
1844-
if let Some((fn_def_id, decl, _)) = self.get_fn_decl(blk.hir_id) {
1844+
if let Some((fn_def_id, decl)) = self.get_fn_decl(blk.hir_id) {
18451845
let ret_sp = decl.output.span();
18461846
if let Some(block_sp) = self.parent_item_span(blk.hir_id) {
18471847
// HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the

Diff for: ‎compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

+36-9
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
7878
// `break` type mismatches provide better context for tail `loop` expressions.
7979
return false;
8080
}
81-
if let Some((fn_id, fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
81+
if let Some((fn_id, fn_decl)) = self.get_fn_decl(blk_id) {
8282
pointing_at_return_type =
83-
self.suggest_missing_return_type(err, fn_decl, expected, found, can_suggest, fn_id);
83+
self.suggest_missing_return_type(err, fn_decl, expected, found, fn_id);
8484
self.suggest_missing_break_or_return_expr(
8585
err, expr, fn_decl, expected, found, blk_id, fn_id,
8686
);
@@ -812,7 +812,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
812812
fn_decl: &hir::FnDecl<'tcx>,
813813
expected: Ty<'tcx>,
814814
found: Ty<'tcx>,
815-
can_suggest: bool,
816815
fn_id: LocalDefId,
817816
) -> bool {
818817
// Can't suggest `->` on a block-like coroutine
@@ -825,14 +824,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
825824
let found =
826825
self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(found));
827826
// Only suggest changing the return type for methods that
828-
// haven't set a return type at all (and aren't `fn main()` or an impl).
827+
// haven't set a return type at all (and aren't `fn main()`, impl or closure).
829828
match &fn_decl.output {
830-
&hir::FnRetTy::DefaultReturn(span) if expected.is_unit() && !can_suggest => {
831-
// `fn main()` must return `()`, do not suggest changing return type
832-
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Unit { span });
833-
return true;
834-
}
829+
// For closure with default returns, don't suggest adding return type
830+
&hir::FnRetTy::DefaultReturn(_) if self.tcx.is_closure_like(fn_id.to_def_id()) => {}
835831
&hir::FnRetTy::DefaultReturn(span) if expected.is_unit() => {
832+
if !self.can_add_return_type(fn_id) {
833+
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Unit { span });
834+
return true;
835+
}
836+
836837
if let Some(found) = found.make_suggestable(self.tcx, false, None) {
837838
err.subdiagnostic(errors::AddReturnTypeSuggestion::Add {
838839
span,
@@ -904,6 +905,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
904905
false
905906
}
906907

908+
/// Checks whether we can add a return type to a function.
909+
/// Assumes given function doesn't have a explicit return type.
910+
fn can_add_return_type(&self, fn_id: LocalDefId) -> bool {
911+
match self.tcx.hir_node_by_def_id(fn_id) {
912+
Node::Item(&hir::Item { ident, .. }) => {
913+
// This is less than ideal, it will not suggest a return type span on any
914+
// method called `main`, regardless of whether it is actually the entry point,
915+
// but it will still present it as the reason for the expected type.
916+
ident.name != sym::main
917+
}
918+
Node::ImplItem(item) => {
919+
// If it doesn't impl a trait, we can add a return type
920+
let Node::Item(&hir::Item {
921+
kind: hir::ItemKind::Impl(&hir::Impl { of_trait, .. }),
922+
..
923+
}) = self.tcx.parent_hir_node(item.hir_id())
924+
else {
925+
unreachable!();
926+
};
927+
928+
of_trait.is_none()
929+
}
930+
_ => true,
931+
}
932+
}
933+
907934
fn try_note_caller_chooses_ty_for_ty_param(
908935
&self,
909936
diag: &mut Diag<'_>,

Diff for: ‎tests/ui/closures/add_semicolon_non_block_closure.rs

-1
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,4 @@ fn main() {
88
foo(|| bar())
99
//~^ ERROR mismatched types [E0308]
1010
//~| HELP consider using a semicolon here
11-
//~| HELP try adding a return type
1211
}

Diff for: ‎tests/ui/closures/add_semicolon_non_block_closure.stderr

-4
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@ help: consider using a semicolon here
88
|
99
LL | foo(|| { bar(); })
1010
| + +++
11-
help: try adding a return type
12-
|
13-
LL | foo(|| -> i32 bar())
14-
| ++++++
1511

1612
error: aborting due to 1 previous error
1713

Diff for: ‎tests/ui/suggestions/try-operator-dont-suggest-semicolon.rs

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

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

Diff for: ‎tests/ui/suggestions/try-operator-dont-suggest-semicolon.stderr

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

169
error[E0308]: mismatched types
17-
--> $DIR/try-operator-dont-suggest-semicolon.rs:17:9
10+
--> $DIR/try-operator-dont-suggest-semicolon.rs:16:9
1811
|
1912
LL | / if true {
2013
LL | |

Diff for: ‎tests/ui/typeck/issue-81943.stderr

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

119
error[E0308]: mismatched types
1210
--> $DIR/issue-81943.rs:8:28
1311
|
1412
LL | f(|x| match x { tmp => { g(tmp) } });
15-
| ^^^^^^ expected `()`, found `i32`
13+
| -------------------^^^^^^----
14+
| | |
15+
| | expected `()`, found `i32`
16+
| expected this to be `()`
1617
|
1718
help: consider using a semicolon here
1819
|
1920
LL | f(|x| match x { tmp => { g(tmp); } });
2021
| +
21-
help: try adding a return type
22+
help: consider using a semicolon here
2223
|
23-
LL | f(|x| -> i32 match x { tmp => { g(tmp) } });
24-
| ++++++
24+
LL | f(|x| match x { tmp => { g(tmp) } };);
25+
| +
2526

2627
error[E0308]: mismatched types
2728
--> $DIR/issue-81943.rs:10:38
2829
|
2930
LL | ($e:expr) => { match $e { x => { g(x) } } }
30-
| ^^^^ expected `()`, found `i32`
31+
| ------------------^^^^----
32+
| | |
33+
| | expected `()`, found `i32`
34+
| expected this to be `()`
3135
LL | }
3236
LL | f(|x| d!(x));
3337
| ----- in this macro invocation
@@ -37,10 +41,10 @@ help: consider using a semicolon here
3741
|
3842
LL | ($e:expr) => { match $e { x => { g(x); } } }
3943
| +
40-
help: try adding a return type
44+
help: consider using a semicolon here
4145
|
42-
LL | f(|x| -> i32 d!(x));
43-
| ++++++
46+
LL | ($e:expr) => { match $e { x => { g(x) } }; }
47+
| +
4448

4549
error: aborting due to 3 previous errors
4650

0 commit comments

Comments
 (0)