Skip to content

Commit 76e070f

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #129260 - wafarm:dont-suggest-closures, r=compiler-errors
Don't suggest adding return type for closures with default return type Follow up of #129223 r? ``@compiler-errors``
2 parents f7f8bdf + 736ab66 commit 76e070f

11 files changed

+74
-73
lines changed

‎compiler/rustc_hir_typeck/src/_match.rs

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

‎compiler/rustc_hir_typeck/src/coercion.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1860,10 +1860,10 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
18601860
};
18611861

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

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

‎compiler/rustc_hir_typeck/src/expr.rs

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

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

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use rustc_middle::{bug, span_bug};
3030
use rustc_session::lint;
3131
use rustc_span::def_id::LocalDefId;
3232
use rustc_span::hygiene::DesugaringKind;
33-
use rustc_span::symbol::{kw, sym};
33+
use rustc_span::symbol::kw;
3434
use rustc_span::Span;
3535
use rustc_target::abi::FieldIdx;
3636
use rustc_trait_selection::error_reporting::infer::need_type_info::TypeAnnotationNeeded;
@@ -859,38 +859,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
859859
)
860860
}
861861

862-
/// Given a `HirId`, return the `HirId` of the enclosing function, its `FnDecl`, and whether a
863-
/// suggestion can be made, `None` otherwise.
862+
/// Given a `HirId`, return the `HirId` of the enclosing function and its `FnDecl`.
864863
pub(crate) fn get_fn_decl(
865864
&self,
866865
blk_id: HirId,
867-
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, bool)> {
866+
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>)> {
868867
// Get enclosing Fn, if it is a function or a trait method, unless there's a `loop` or
869868
// `while` before reaching it, as block tail returns are not available in them.
870869
self.tcx.hir().get_fn_id_for_return_block(blk_id).and_then(|item_id| {
871870
match self.tcx.hir_node(item_id) {
872871
Node::Item(&hir::Item {
873-
ident,
874-
kind: hir::ItemKind::Fn(ref sig, ..),
875-
owner_id,
876-
..
877-
}) => {
878-
// This is less than ideal, it will not suggest a return type span on any
879-
// method called `main`, regardless of whether it is actually the entry point,
880-
// but it will still present it as the reason for the expected type.
881-
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
882-
}
872+
kind: hir::ItemKind::Fn(ref sig, ..), owner_id, ..
873+
}) => Some((owner_id.def_id, sig.decl)),
883874
Node::TraitItem(&hir::TraitItem {
884875
kind: hir::TraitItemKind::Fn(ref sig, ..),
885876
owner_id,
886877
..
887-
}) => Some((owner_id.def_id, sig.decl, true)),
888-
// FIXME: Suggestable if this is not a trait implementation
878+
}) => Some((owner_id.def_id, sig.decl)),
889879
Node::ImplItem(&hir::ImplItem {
890880
kind: hir::ImplItemKind::Fn(ref sig, ..),
891881
owner_id,
892882
..
893-
}) => Some((owner_id.def_id, sig.decl, false)),
883+
}) => Some((owner_id.def_id, sig.decl)),
894884
Node::Expr(&hir::Expr {
895885
hir_id,
896886
kind: hir::ExprKind::Closure(&hir::Closure { def_id, kind, fn_decl, .. }),
@@ -901,33 +891,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
901891
// FIXME(async_closures): Implement this.
902892
return None;
903893
}
904-
hir::ClosureKind::Closure => Some((def_id, fn_decl, true)),
894+
hir::ClosureKind::Closure => Some((def_id, fn_decl)),
905895
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
906896
_,
907897
hir::CoroutineSource::Fn,
908898
)) => {
909-
let (ident, sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
899+
let (sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
910900
Node::Item(&hir::Item {
911-
ident,
912901
kind: hir::ItemKind::Fn(ref sig, ..),
913902
owner_id,
914903
..
915-
}) => (ident, sig, owner_id),
904+
}) => (sig, owner_id),
916905
Node::TraitItem(&hir::TraitItem {
917-
ident,
918906
kind: hir::TraitItemKind::Fn(ref sig, ..),
919907
owner_id,
920908
..
921-
}) => (ident, sig, owner_id),
909+
}) => (sig, owner_id),
922910
Node::ImplItem(&hir::ImplItem {
923-
ident,
924911
kind: hir::ImplItemKind::Fn(ref sig, ..),
925912
owner_id,
926913
..
927-
}) => (ident, sig, owner_id),
914+
}) => (sig, owner_id),
928915
_ => return None,
929916
};
930-
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
917+
Some((owner_id.def_id, sig.decl))
931918
}
932919
_ => None,
933920
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1873,7 +1873,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
18731873
// that highlight errors inline.
18741874
let mut sp = blk.span;
18751875
let mut fn_span = None;
1876-
if let Some((fn_def_id, decl, _)) = self.get_fn_decl(blk.hir_id) {
1876+
if let Some((fn_def_id, decl)) = self.get_fn_decl(blk.hir_id) {
18771877
let ret_sp = decl.output.span();
18781878
if let Some(block_sp) = self.parent_item_span(blk.hir_id) {
18791879
// HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the

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

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
7979
// `break` type mismatches provide better context for tail `loop` expressions.
8080
return false;
8181
}
82-
if let Some((fn_id, fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
82+
if let Some((fn_id, fn_decl)) = self.get_fn_decl(blk_id) {
8383
pointing_at_return_type =
84-
self.suggest_missing_return_type(err, fn_decl, expected, found, can_suggest, fn_id);
84+
self.suggest_missing_return_type(err, fn_decl, expected, found, fn_id);
8585
self.suggest_missing_break_or_return_expr(
8686
err, expr, fn_decl, expected, found, blk_id, fn_id,
8787
);
@@ -813,7 +813,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
813813
fn_decl: &hir::FnDecl<'tcx>,
814814
expected: Ty<'tcx>,
815815
found: Ty<'tcx>,
816-
can_suggest: bool,
817816
fn_id: LocalDefId,
818817
) -> bool {
819818
// Can't suggest `->` on a block-like coroutine
@@ -826,28 +825,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
826825
let found =
827826
self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(found));
828827
// Only suggest changing the return type for methods that
829-
// haven't set a return type at all (and aren't `fn main()` or an impl).
828+
// haven't set a return type at all (and aren't `fn main()`, impl or closure).
830829
match &fn_decl.output {
831-
&hir::FnRetTy::DefaultReturn(span) if expected.is_unit() && !can_suggest => {
832-
// `fn main()` must return `()`, do not suggest changing return type
833-
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Unit { span });
834-
return true;
835-
}
830+
// For closure with default returns, don't suggest adding return type
831+
&hir::FnRetTy::DefaultReturn(_) if self.tcx.is_closure_like(fn_id.to_def_id()) => {}
836832
&hir::FnRetTy::DefaultReturn(span) if expected.is_unit() => {
837-
if let Some(found) = found.make_suggestable(self.tcx, false, None) {
833+
if !self.can_add_return_type(fn_id) {
834+
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Unit { span });
835+
} else if let Some(found) = found.make_suggestable(self.tcx, false, None) {
838836
err.subdiagnostic(errors::AddReturnTypeSuggestion::Add {
839837
span,
840838
found: found.to_string(),
841839
});
842-
return true;
843840
} else if let Some(sugg) = suggest_impl_trait(self, self.param_env, found) {
844841
err.subdiagnostic(errors::AddReturnTypeSuggestion::Add { span, found: sugg });
845-
return true;
846842
} else {
847843
// FIXME: if `found` could be `impl Iterator` we should suggest that.
848844
err.subdiagnostic(errors::AddReturnTypeSuggestion::MissingHere { span });
849-
return true;
850845
}
846+
847+
return true;
851848
}
852849
hir::FnRetTy::Return(hir_ty) => {
853850
if let hir::TyKind::OpaqueDef(item_id, ..) = hir_ty.kind
@@ -905,6 +902,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
905902
false
906903
}
907904

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

‎tests/ui/closures/add_semicolon_non_block_closure.rs

Lines changed: 0 additions & 1 deletion
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
}

‎tests/ui/closures/add_semicolon_non_block_closure.stderr

Lines changed: 0 additions & 4 deletions
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

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

Lines changed: 0 additions & 1 deletion
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`

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

Lines changed: 5 additions & 12 deletions
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 | |

‎tests/ui/typeck/issue-81943.stderr

Lines changed: 15 additions & 11 deletions
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)