Rollup merge of rust-lang#109248 - compiler-errors:get_fn_decl-aaa, r=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.

Author: matthiaskrgr

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")))

compiler/rustc_hir_typeck/src/coercion.rs

@@ -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 {
@@ -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()

compiler/rustc_hir_typeck/src/expr.rs

@@ -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,

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))
         })
     }
 

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

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,

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() {}

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`.