Skip to content

Commit

Permalink
Inline get_node_fn_decl into get_fn_decl, simplify/explain logic in r…
Browse files Browse the repository at this point in the history 
…eport_return_mismatched_types
  • Loading branch information
@compiler-errors
compiler-errors committed Apr 26, 2024
1 parent 72768cd commit b2eadba
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 119 deletions.
33 changes: 10 additions & 23 deletions compiler/rustc_hir_typeck/src/coercion.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1994,39 +1994,26 @@ 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(parent_id)
&& !due_to_block
{
fcx.suggest_missing_return_type(&mut err, fn_decl, expected, found, can_suggest, fn_id);
}

let mut parent_id = fcx.tcx.hir().get_parent_item(block_or_return_id).def_id;
let mut parent_item = fcx.tcx.hir_node_by_def_id(parent_id);
// When suggesting return, we need to account for closures and async blocks, not just items.
// FIXME: fix get_fn_decl to be async block aware, use get_fn_decl results above
for (_, node) in fcx.tcx.hir().parent_iter(block_or_return_id) {
match node {
hir::Node::Expr(&hir::Expr {
kind: hir::ExprKind::Closure(hir::Closure { def_id, .. }),
..
}) => {
parent_item = node;
parent_id = *def_id;
break;
}
hir::Node::Item(_) | hir::Node::TraitItem(_) | hir::Node::ImplItem(_) => break,
_ => {}
}
}

if let Some(expr) = expression
&& let Some(fn_decl) = parent_item.fn_decl()
&& due_to_block
// If this is due to a block, then maybe we forgot a `return`/`break`.
if due_to_block
&& let Some(expr) = expression
&& let Some((parent_fn_decl, parent_id)) = fcx
.tcx
.hir()
.parent_iter(block_or_return_id)
.find_map(|(_, node)| Some((node.fn_decl()?, node.associated_body()?.0)))
{
fcx.suggest_missing_break_or_return_expr(
&mut err,
expr,
fn_decl,
parent_fn_decl,
expected,
found,
block_or_return_id,
Expand Down
142 changes: 67 additions & 75 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use rustc_middle::ty::{GenericArgKind, GenericArgsRef, UserArgs, UserSelfTy};
use rustc_session::lint;
use rustc_span::def_id::LocalDefId;
use rustc_span::hygiene::DesugaringKind;
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::symbol::{kw, sym};
use rustc_span::Span;
use rustc_target::abi::FieldIdx;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
Expand Down Expand Up @@ -860,76 +860,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
}

/// Given a function `Node`, return its `HirId` and `FnDecl` if it exists. Given a closure
/// that is the child of a function, return that function's `HirId` and `FnDecl` instead.
/// This may seem confusing at first, but this is used in diagnostics for `async fn`,
/// for example, where most of the type checking actually happens within a nested closure,
/// but we often want access to the parent function's signature.
///
/// Otherwise, return false.
pub(crate) fn get_node_fn_decl(
&self,
node: Node<'tcx>,
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, Ident, bool)> {
match node {
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, ident.name != sym::main))
}
Node::TraitItem(&hir::TraitItem {
ident,
kind: hir::TraitItemKind::Fn(ref sig, ..),
owner_id,
..
}) => Some((owner_id.def_id, &sig.decl, ident, true)),
Node::ImplItem(&hir::ImplItem {
ident,
kind: hir::ImplItemKind::Fn(ref sig, ..),
owner_id,
..
}) => Some((owner_id.def_id, &sig.decl, ident, false)),
Node::Expr(&hir::Expr {
hir_id,
kind:
hir::ExprKind::Closure(hir::Closure {
kind: hir::ClosureKind::Coroutine(..), ..
}),
..
}) => {
let (ident, 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),
Node::TraitItem(&hir::TraitItem {
ident,
kind: hir::TraitItemKind::Fn(ref sig, ..),
owner_id,
..
}) => (ident, sig, owner_id),
Node::ImplItem(&hir::ImplItem {
ident,
kind: hir::ImplItemKind::Fn(ref sig, ..),
owner_id,
..
}) => (ident, sig, owner_id),
_ => return None,
};
Some((owner_id.def_id, &sig.decl, ident, ident.name != sym::main))
}
_ => None,
}
}

/// Given a `HirId`, return the `HirId` of the enclosing function, its `FnDecl`, and whether a
/// suggestion can be made, `None` otherwise.
pub fn get_fn_decl(
Expand All @@ -938,10 +868,72 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, bool)> {
// 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(|blk_id| {
let parent = self.tcx.hir_node(blk_id);
self.get_node_fn_decl(parent)
.map(|(fn_id, fn_decl, _, is_main)| (fn_id, fn_decl, is_main))
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))
}
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
Node::ImplItem(&hir::ImplItem {
kind: hir::ImplItemKind::Fn(ref sig, ..),
owner_id,
..
}) => Some((owner_id.def_id, sig.decl, false)),
Node::Expr(&hir::Expr {
hir_id,
kind: hir::ExprKind::Closure(&hir::Closure { def_id, kind, fn_decl, .. }),
..
}) => {
match kind {
hir::ClosureKind::CoroutineClosure(_) => {
// FIXME(async_closures): Implement this.
return None;
}
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
_,
hir::CoroutineSource::Fn,
)) => {
let (ident, 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),
Node::TraitItem(&hir::TraitItem {
ident,
kind: hir::TraitItemKind::Fn(ref sig, ..),
owner_id,
..
}) => (ident, sig, owner_id),
Node::ImplItem(&hir::ImplItem {
ident,
kind: hir::ImplItemKind::Fn(ref sig, ..),
owner_id,
..
}) => (ident, sig, owner_id),
_ => return None,
};
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
}
_ => Some((def_id, fn_decl, true)),
}
}
_ => None,
}
})
}

Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
can_suggest: bool,
fn_id: LocalDefId,
) -> bool {
// Can't suggest `->` on a block-like coroutine
if let Some(hir::CoroutineKind::Desugared(_, hir::CoroutineSource::Block)) =
self.tcx.coroutine_kind(fn_id)
{
return false;
}

let found =
self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(found));
// Only suggest changing the return type for methods that
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/async-await/dont-ice-for-type-mismatch-in-closure-in-async.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ LL | | }
error[E0308]: mismatched types
--> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:12:9
|
LL | call(|| -> Option<()> {
| ---------- expected `Option<()>` because of return type
...
LL | true
| ^^^^ expected `Option<()>`, found `bool`
|
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/closures/closure-return-type-mismatch.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ LL | return "test";
error[E0308]: mismatched types
--> $DIR/closure-return-type-mismatch.rs:12:20
|
LL | || -> bool {
| ---- expected `bool` because of return type
LL | if false {
LL | return "hello"
| ^^^^^^^ expected `bool`, found `&str`

Expand Down
4 changes: 3 additions & 1 deletion tests/ui/impl-trait/issue-99914.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ error[E0308]: mismatched types
--> $DIR/issue-99914.rs:9:27
|
LL | t.and_then(|t| -> _ { bar(t) });
| ^^^^^^ expected `Result<_, Error>`, found future
| - ^^^^^^ expected `Result<_, Error>`, found future
| |
| expected `Result<_, Error>` because of return type
|
help: try wrapping the expression in `Ok`
|
Expand Down
1 change: 1 addition & 0 deletions tests/ui/suggestions/try-operator-dont-suggest-semicolon.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

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

error[E0308]: mismatched types
--> $DIR/try-operator-dont-suggest-semicolon.rs:16:9
--> $DIR/try-operator-dont-suggest-semicolon.rs:17:9
|
LL | / if true {
LL | |
Expand Down
26 changes: 11 additions & 15 deletions tests/ui/typeck/issue-81943.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,32 @@ error[E0308]: mismatched types
--> $DIR/issue-81943.rs:7:9
|
LL | f(|x| lib::d!(x));
| ^^^^^^^^^^ expected `()`, found `i32`
| -^^^^^^^^^^ expected `()`, found `i32`
| |
| help: try adding a return type: `-> 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 this to be `()`
| ^^^^^^ expected `()`, found `i32`
|
help: consider using a semicolon here
|
LL | f(|x| match x { tmp => { g(tmp); } });
| +
help: consider using a semicolon here
help: try adding a return type
|
LL | f(|x| match x { tmp => { g(tmp) } };);
| +
LL | f(|x| -> i32 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 this to be `()`
| ^^^^ expected `()`, found `i32`
LL | }
LL | f(|x| d!(x));
| ----- in this macro invocation
Expand All @@ -41,10 +37,10 @@ help: consider using a semicolon here
|
LL | ($e:expr) => { match $e { x => { g(x); } } }
| +
help: consider using a semicolon here
help: try adding a return type
|
LL | ($e:expr) => { match $e { x => { g(x) } }; }
| +
LL | f(|x| -> i32 d!(x));
| ++++++

error: aborting due to 3 previous errors

Expand Down

0 comments on commit b2eadba

Please sign in to comment.