Skip to content

Commit

Permalink
Don't suggest adding return type for closures with default return type
Browse files Browse the repository at this point in the history
  • Loading branch information
wafarm committed Aug 28, 2024
1 parent d9a2cc4 commit 736ab66
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 73 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
{
// check that the `if` expr without `else` is the fn body's expr
if expr.span == sp {
return self.get_fn_decl(hir_id).map(|(_, fn_decl, _)| {
return self.get_fn_decl(hir_id).map(|(_, fn_decl)| {
let (ty, span) = match fn_decl.output {
hir::FnRetTy::DefaultReturn(span) => ("()".to_string(), span),
hir::FnRetTy::Return(ty) => (ty_to_string(&self.tcx, ty), ty.span),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1859,10 +1859,10 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
};

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

// If this is due to a block, then maybe we forgot a `return`/`break`.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.ret_coercion_span.set(Some(expr.span));
}
let cause = self.cause(expr.span, ObligationCauseCode::ReturnNoExpression);
if let Some((_, fn_decl, _)) = self.get_fn_decl(expr.hir_id) {
if let Some((_, fn_decl)) = self.get_fn_decl(expr.hir_id) {
coercion.coerce_forced_unit(
self,
&cause,
Expand Down
39 changes: 13 additions & 26 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use rustc_middle::{bug, span_bug};
use rustc_session::lint;
use rustc_span::def_id::LocalDefId;
use rustc_span::hygiene::DesugaringKind;
use rustc_span::symbol::{kw, sym};
use rustc_span::symbol::kw;
use rustc_span::Span;
use rustc_target::abi::FieldIdx;
use rustc_trait_selection::error_reporting::infer::need_type_info::TypeAnnotationNeeded;
Expand Down Expand Up @@ -895,38 +895,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
}

/// Given a `HirId`, return the `HirId` of the enclosing function, its `FnDecl`, and whether a
/// suggestion can be made, `None` otherwise.
/// Given a `HirId`, return the `HirId` of the enclosing function and its `FnDecl`.
pub(crate) fn get_fn_decl(
&self,
blk_id: HirId,
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, bool)> {
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>)> {
// Get enclosing Fn, if it is a function or a trait method, unless there's a `loop` or
// `while` before reaching it, as block tail returns are not available in them.
self.tcx.hir().get_fn_id_for_return_block(blk_id).and_then(|item_id| {
match self.tcx.hir_node(item_id) {
Node::Item(&hir::Item {
ident,
kind: hir::ItemKind::Fn(ref sig, ..),
owner_id,
..
}) => {
// This is less than ideal, it will not suggest a return type span on any
// method called `main`, regardless of whether it is actually the entry point,
// but it will still present it as the reason for the expected type.
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
}
kind: hir::ItemKind::Fn(ref sig, ..), owner_id, ..
}) => Some((owner_id.def_id, sig.decl)),
Node::TraitItem(&hir::TraitItem {
kind: hir::TraitItemKind::Fn(ref sig, ..),
owner_id,
..
}) => Some((owner_id.def_id, sig.decl, true)),
// FIXME: Suggestable if this is not a trait implementation
}) => Some((owner_id.def_id, sig.decl)),
Node::ImplItem(&hir::ImplItem {
kind: hir::ImplItemKind::Fn(ref sig, ..),
owner_id,
..
}) => Some((owner_id.def_id, sig.decl, false)),
}) => Some((owner_id.def_id, sig.decl)),
Node::Expr(&hir::Expr {
hir_id,
kind: hir::ExprKind::Closure(&hir::Closure { def_id, kind, fn_decl, .. }),
Expand All @@ -937,33 +927,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// FIXME(async_closures): Implement this.
return None;
}
hir::ClosureKind::Closure => Some((def_id, fn_decl, true)),
hir::ClosureKind::Closure => Some((def_id, fn_decl)),
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
_,
hir::CoroutineSource::Fn,
)) => {
let (ident, sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
let (sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
Node::Item(&hir::Item {
ident,
kind: hir::ItemKind::Fn(ref sig, ..),
owner_id,
..
}) => (ident, sig, owner_id),
}) => (sig, owner_id),
Node::TraitItem(&hir::TraitItem {
ident,
kind: hir::TraitItemKind::Fn(ref sig, ..),
owner_id,
..
}) => (ident, sig, owner_id),
}) => (sig, owner_id),
Node::ImplItem(&hir::ImplItem {
ident,
kind: hir::ImplItemKind::Fn(ref sig, ..),
owner_id,
..
}) => (ident, sig, owner_id),
}) => (sig, owner_id),
_ => return None,
};
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
Some((owner_id.def_id, sig.decl))
}
_ => None,
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1841,7 +1841,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// that highlight errors inline.
let mut sp = blk.span;
let mut fn_span = None;
if let Some((fn_def_id, decl, _)) = self.get_fn_decl(blk.hir_id) {
if let Some((fn_def_id, decl)) = self.get_fn_decl(blk.hir_id) {
let ret_sp = decl.output.span();
if let Some(block_sp) = self.parent_item_span(blk.hir_id) {
// HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the
Expand Down
49 changes: 36 additions & 13 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// `break` type mismatches provide better context for tail `loop` expressions.
return false;
}
if let Some((fn_id, fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
if let Some((fn_id, fn_decl)) = self.get_fn_decl(blk_id) {
pointing_at_return_type =
self.suggest_missing_return_type(err, fn_decl, expected, found, can_suggest, fn_id);
self.suggest_missing_return_type(err, fn_decl, expected, found, fn_id);
self.suggest_missing_break_or_return_expr(
err, expr, fn_decl, expected, found, blk_id, fn_id,
);
Expand Down Expand Up @@ -812,7 +812,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn_decl: &hir::FnDecl<'tcx>,
expected: Ty<'tcx>,
found: Ty<'tcx>,
can_suggest: bool,
fn_id: LocalDefId,
) -> bool {
// Can't suggest `->` on a block-like coroutine
Expand All @@ -825,28 +824,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let found =
self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(found));
// Only suggest changing the return type for methods that
// haven't set a return type at all (and aren't `fn main()` or an impl).
// haven't set a return type at all (and aren't `fn main()`, impl or closure).
match &fn_decl.output {
&hir::FnRetTy::DefaultReturn(span) if expected.is_unit() && !can_suggest => {
// `fn main()` must return `()`, do not suggest changing return type
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Unit { span });
return true;
}
// For closure with default returns, don't suggest adding return type
&hir::FnRetTy::DefaultReturn(_) if self.tcx.is_closure_like(fn_id.to_def_id()) => {}
&hir::FnRetTy::DefaultReturn(span) if expected.is_unit() => {
if let Some(found) = found.make_suggestable(self.tcx, false, None) {
if !self.can_add_return_type(fn_id) {
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Unit { span });
} else if let Some(found) = found.make_suggestable(self.tcx, false, None) {
err.subdiagnostic(errors::AddReturnTypeSuggestion::Add {
span,
found: found.to_string(),
});
return true;
} else if let Some(sugg) = suggest_impl_trait(self, self.param_env, found) {
err.subdiagnostic(errors::AddReturnTypeSuggestion::Add { span, found: sugg });
return true;
} else {
// FIXME: if `found` could be `impl Iterator` we should suggest that.
err.subdiagnostic(errors::AddReturnTypeSuggestion::MissingHere { span });
return true;
}

return true;
}
hir::FnRetTy::Return(hir_ty) => {
if let hir::TyKind::OpaqueDef(item_id, ..) = hir_ty.kind
Expand Down Expand Up @@ -904,6 +901,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false
}

/// Checks whether we can add a return type to a function.
/// Assumes given function doesn't have a explicit return type.
fn can_add_return_type(&self, fn_id: LocalDefId) -> bool {
match self.tcx.hir_node_by_def_id(fn_id) {
Node::Item(&hir::Item { ident, .. }) => {
// This is less than ideal, it will not suggest a return type span on any
// method called `main`, regardless of whether it is actually the entry point,
// but it will still present it as the reason for the expected type.
ident.name != sym::main
}
Node::ImplItem(item) => {
// If it doesn't impl a trait, we can add a return type
let Node::Item(&hir::Item {
kind: hir::ItemKind::Impl(&hir::Impl { of_trait, .. }),
..
}) = self.tcx.parent_hir_node(item.hir_id())
else {
unreachable!();
};

of_trait.is_none()
}
_ => true,
}
}

fn try_note_caller_chooses_ty_for_ty_param(
&self,
diag: &mut Diag<'_>,
Expand Down
1 change: 0 additions & 1 deletion tests/ui/closures/add_semicolon_non_block_closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@ fn main() {
foo(|| bar())
//~^ ERROR mismatched types [E0308]
//~| HELP consider using a semicolon here
//~| HELP try adding a return type
}
4 changes: 0 additions & 4 deletions tests/ui/closures/add_semicolon_non_block_closure.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ help: consider using a semicolon here
|
LL | foo(|| { bar(); })
| + +++
help: try adding a return type
|
LL | foo(|| -> i32 bar())
| ++++++

error: aborting due to 1 previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

fn main() -> Result<(), ()> {
a(|| {
//~^ HELP: try adding a return type
b()
//~^ ERROR: mismatched types [E0308]
//~| NOTE: expected `()`, found `i32`
Expand Down
17 changes: 5 additions & 12 deletions tests/ui/suggestions/try-operator-dont-suggest-semicolon.stderr
Original file line number Diff line number Diff line change
@@ -1,20 +1,13 @@
error[E0308]: mismatched types
--> $DIR/try-operator-dont-suggest-semicolon.rs:7:9
--> $DIR/try-operator-dont-suggest-semicolon.rs:6:9
|
LL | b()
| ^^^ expected `()`, found `i32`
|
help: consider using a semicolon here
|
LL | b();
| +
help: try adding a return type
|
LL | a(|| -> i32 {
| ++++++
| ^^^- help: consider using a semicolon here: `;`
| |
| expected `()`, found `i32`

error[E0308]: mismatched types
--> $DIR/try-operator-dont-suggest-semicolon.rs:17:9
--> $DIR/try-operator-dont-suggest-semicolon.rs:16:9
|
LL | / if true {
LL | |
Expand Down
26 changes: 15 additions & 11 deletions tests/ui/typeck/issue-81943.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,36 @@ error[E0308]: mismatched types
--> $DIR/issue-81943.rs:7:9
|
LL | f(|x| lib::d!(x));
| -^^^^^^^^^^ expected `()`, found `i32`
| |
| help: try adding a return type: `-> i32`
| ^^^^^^^^^^ expected `()`, found `i32`
|
= note: this error originates in the macro `lib::d` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
--> $DIR/issue-81943.rs:8:28
|
LL | f(|x| match x { tmp => { g(tmp) } });
| ^^^^^^ expected `()`, found `i32`
| -------------------^^^^^^----
| | |
| | expected `()`, found `i32`
| expected this to be `()`
|
help: consider using a semicolon here
|
LL | f(|x| match x { tmp => { g(tmp); } });
| +
help: try adding a return type
help: consider using a semicolon here
|
LL | f(|x| -> i32 match x { tmp => { g(tmp) } });
| ++++++
LL | f(|x| match x { tmp => { g(tmp) } };);
| +

error[E0308]: mismatched types
--> $DIR/issue-81943.rs:10:38
|
LL | ($e:expr) => { match $e { x => { g(x) } } }
| ^^^^ expected `()`, found `i32`
| ------------------^^^^----
| | |
| | expected `()`, found `i32`
| expected this to be `()`
LL | }
LL | f(|x| d!(x));
| ----- in this macro invocation
Expand All @@ -37,10 +41,10 @@ help: consider using a semicolon here
|
LL | ($e:expr) => { match $e { x => { g(x); } } }
| +
help: try adding a return type
help: consider using a semicolon here
|
LL | f(|x| -> i32 d!(x));
| ++++++
LL | ($e:expr) => { match $e { x => { g(x) } }; }
| +

error: aborting due to 3 previous errors

Expand Down

0 comments on commit 736ab66

Please sign in to comment.