Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't suggest adding return type for closures with default return type #129260

Merged
merged 1 commit into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading