From 08c913279fc21e57acd210a98dfdfd740e86b565 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 17 Mar 2023 03:14:27 +0000 Subject: [PATCH] Pass the right HIR back from get_fn_decl --- compiler/rustc_hir_typeck/src/_match.rs | 2 +- compiler/rustc_hir_typeck/src/coercion.rs | 19 +++--- compiler/rustc_hir_typeck/src/expr.rs | 2 +- .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 61 +++++++++++++------ .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 2 +- .../src/fn_ctxt/suggestions.rs | 3 +- .../suggest-ret-on-async-w-late.rs | 11 ++++ .../suggest-ret-on-async-w-late.stderr | 11 ++++ 8 files changed, 75 insertions(+), 36 deletions(-) create mode 100644 tests/ui/suggestions/suggest-ret-on-async-w-late.rs create mode 100644 tests/ui/suggestions/suggest-ret-on-async-w-late.stderr diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index e19ef2ff3bf48..035ccf30b2462 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/compiler/rustc_hir_typeck/src/_match.rs @@ -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"))) diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 3d6274ede8146..a27905ea46c94 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -1722,12 +1722,13 @@ 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, @@ -1735,7 +1736,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { expected, found, can_suggest, - fcx.tcx.hir().get_parent_item(id).into(), + fn_id, ); } if !pointing_at_return_type { @@ -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, ); } @@ -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() diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index afef331ec1d93..d64b5728f3251 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -799,7 +799,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, diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index e539693402af9..8455076de5634 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -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)) }) } diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 7064ff6538487..61338ac613aea 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -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 diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index 690d8a238261a..7a09ea40d7974 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -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, diff --git a/tests/ui/suggestions/suggest-ret-on-async-w-late.rs b/tests/ui/suggestions/suggest-ret-on-async-w-late.rs new file mode 100644 index 0000000000000..459b94f943b50 --- /dev/null +++ b/tests/ui/suggestions/suggest-ret-on-async-w-late.rs @@ -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() {} diff --git a/tests/ui/suggestions/suggest-ret-on-async-w-late.stderr b/tests/ui/suggestions/suggest-ret-on-async-w-late.stderr new file mode 100644 index 0000000000000..bff864b222bff --- /dev/null +++ b/tests/ui/suggestions/suggest-ret-on-async-w-late.stderr @@ -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`.