Skip to content

Commit

Permalink
Rollup merge of rust-lang#109248 - compiler-errors:get_fn_decl-aaa, r…
Browse files Browse the repository at this point in the history
…=WaffleLapkin

Pass the right HIR back from `get_fn_decl`

Fixes rust-lang#109232

Makes sure that the `fn_id: HirId` that we pass to `suggest_missing_return_type` matches up with the `fn_decl: hir::FnDecl` that we pass to it, so the late-bound vars that we fetch from the former match up with the types in the latter...

This HIR suggestion code really needs a big refactor. I've tried to do it in the past (a couple of attempts), but it's a super tangled mess. It really shouldn't be passing around things like `hir::Node` and just deal with `LocalDefId`s everywhere... Anyways, I'd rather fix this ICE, now.
  • Loading branch information
matthiaskrgr authored Mar 17, 2023
2 parents 195cefa + 08c9132 commit b83b925
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 36 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 @@ -299,7 +299,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).and_then(|(fn_decl, _)| {
return self.get_fn_decl(hir_id).and_then(|(_, fn_decl, _)| {
let span = fn_decl.output.span();
let snippet = self.tcx.sess.source_map().span_to_snippet(span).ok()?;
Some((span, format!("expected `{snippet}` because of this return type")))
Expand Down
19 changes: 7 additions & 12 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1722,20 +1722,21 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
fcx.suggest_semicolon_at_end(cond_expr.span, &mut err);
}
}
fcx.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main))
fcx.get_node_fn_decl(parent)
.map(|(fn_id, fn_decl, _, is_main)| (fn_id, fn_decl, is_main))
} else {
fcx.get_fn_decl(parent_id)
};

if let Some((fn_decl, can_suggest)) = fn_decl {
if let Some((fn_id, fn_decl, can_suggest)) = fn_decl {
if blk_id.is_none() {
pointing_at_return_type |= fcx.suggest_missing_return_type(
&mut err,
&fn_decl,
expected,
found,
can_suggest,
fcx.tcx.hir().get_parent_item(id).into(),
fn_id,
);
}
if !pointing_at_return_type {
Expand All @@ -1746,17 +1747,11 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
let parent_id = fcx.tcx.hir().get_parent_item(id);
let parent_item = fcx.tcx.hir().get_by_def_id(parent_id.def_id);

if let (Some(expr), Some(_), Some((fn_decl, _, _))) =
if let (Some(expr), Some(_), Some((fn_id, fn_decl, _, _))) =
(expression, blk_id, fcx.get_node_fn_decl(parent_item))
{
fcx.suggest_missing_break_or_return_expr(
&mut err,
expr,
fn_decl,
expected,
found,
id,
parent_id.into(),
&mut err, expr, fn_decl, expected, found, id, fn_id,
);
}

Expand Down Expand Up @@ -1882,7 +1877,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
}

fn is_return_ty_unsized<'a>(&self, fcx: &FnCtxt<'a, 'tcx>, blk_id: hir::HirId) -> bool {
if let Some((fn_decl, _)) = fcx.get_fn_decl(blk_id)
if let Some((_, fn_decl, _)) = fcx.get_fn_decl(blk_id)
&& let hir::FnRetTy::Return(ty) = fn_decl.output
&& let ty = fcx.astconv().ast_ty_to_ty( ty)
&& let ty::Dynamic(..) = ty.kind()
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 @@ -788,7 +788,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
61 changes: 42 additions & 19 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,51 +898,74 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
}

/// Given a function `Node`, return its `FnDecl` if it exists, or `None` otherwise.
/// 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(in super::super) fn get_node_fn_decl(
&self,
node: Node<'tcx>,
) -> Option<(&'tcx hir::FnDecl<'tcx>, Ident, bool)> {
) -> Option<(hir::HirId, &'tcx hir::FnDecl<'tcx>, Ident, bool)> {
match node {
Node::Item(&hir::Item { ident, kind: hir::ItemKind::Fn(ref sig, ..), .. }) => {
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((&sig.decl, ident, ident.name != sym::main))
Some((
hir::HirId::make_owner(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((&sig.decl, ident, true)),
}) => Some((hir::HirId::make_owner(owner_id.def_id), &sig.decl, ident, true)),
Node::ImplItem(&hir::ImplItem {
ident,
kind: hir::ImplItemKind::Fn(ref sig, ..),
owner_id,
..
}) => Some((&sig.decl, ident, false)),
Node::Expr(&hir::Expr {
hir_id,
kind: hir::ExprKind::Closure(..),
..
}) if let Some(Node::Item(&hir::Item {
}) => Some((hir::HirId::make_owner(owner_id.def_id), &sig.decl, ident, false)),
Node::Expr(&hir::Expr { hir_id, kind: hir::ExprKind::Closure(..), .. })
if let Some(Node::Item(&hir::Item {
ident,
kind: hir::ItemKind::Fn(ref sig, ..),
owner_id,
..
})) = self.tcx.hir().find_parent(hir_id) => Some((
hir::HirId::make_owner(owner_id.def_id),
&sig.decl,
ident,
kind: hir::ItemKind::Fn(ref sig, ..),
..
})) = self.tcx.hir().find_parent(hir_id) => {
Some((&sig.decl, ident, ident.name != sym::main))
},
ident.name != sym::main,
)),
_ => None,
}
}

/// Given a `HirId`, return the `FnDecl` of the method it is enclosed by and whether a
/// 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(&self, blk_id: hir::HirId) -> Option<(&'tcx hir::FnDecl<'tcx>, bool)> {
pub fn get_fn_decl(
&self,
blk_id: hir::HirId,
) -> Option<(hir::HirId, &'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_return_block(blk_id).and_then(|blk_id| {
let parent = self.tcx.hir().get(blk_id);
self.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main))
self.get_node_fn_decl(parent)
.map(|(fn_id, fn_decl, _, is_main)| (fn_id, fn_decl, is_main))
})
}

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 @@ -1669,7 +1669,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Given a function block's `HirId`, returns its `FnDecl` if it exists, or `None` otherwise.
fn get_parent_fn_decl(&self, blk_id: hir::HirId) -> Option<(&'tcx hir::FnDecl<'tcx>, Ident)> {
let parent = self.tcx.hir().get_by_def_id(self.tcx.hir().get_parent_item(blk_id).def_id);
self.get_node_fn_decl(parent).map(|(fn_decl, ident, _)| (fn_decl, ident))
self.get_node_fn_decl(parent).map(|(_, fn_decl, ident, _)| (fn_decl, ident))
}

/// If `expr` is a `match` expression that has only one non-`!` arm, use that arm's tail
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let expr = expr.peel_drop_temps();
self.suggest_missing_semicolon(err, expr, expected, false);
let mut pointing_at_return_type = false;
if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
let fn_id = self.tcx.hir().get_return_block(blk_id).unwrap();
if let Some((fn_id, fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
pointing_at_return_type = self.suggest_missing_return_type(
err,
&fn_decl,
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/suggestions/suggest-ret-on-async-w-late.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// edition: 2021

// Make sure we don't ICE when suggesting a return type
// for an async fn that has late-bound vars...

async fn ice(_: &i32) {
true
//~^ ERROR mismatched types
}

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/suggestions/suggest-ret-on-async-w-late.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0308]: mismatched types
--> $DIR/suggest-ret-on-async-w-late.rs:7:5
|
LL | async fn ice(_: &i32) {
| - help: try adding a return type: `-> bool`
LL | true
| ^^^^ expected `()`, found `bool`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.

0 comments on commit b83b925

Please sign in to comment.