Rollup merge of #123804 - compiler-errors:podcrab-fix, r=jieyouxu

Stop using `HirId` for fn-like parents since closures are not `OwnerNode`s

This is a minimal fix for #123273.

I'm overall pretty disappointed w/ the state of this code; although it's "just diagnostics", it still should be maintainable and understandable and neither of those are true. I believe this code really needs some major overhauling before anything more should be added to it, because there are subtle invariants that are being exercised and subsequently broken all over the place, and I don't think we should just paper over them (e.g.) by delaying bugs or things like that. I wouldn't be surprised if fixing up this code would also yield better diagnostics.

Author: matthiaskrgr

compiler/rustc_hir_typeck/src/coercion.rs

@@ -2004,16 +2004,17 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
             }
         }
 
-        let parent_id = fcx.tcx.hir().get_parent_item(id);
-        let mut parent_item = fcx.tcx.hir_node_by_def_id(parent_id.def_id);
+        let mut parent_id = fcx.tcx.hir().get_parent_item(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.
         for (_, node) in fcx.tcx.hir().parent_iter(id) {
             match node {
                 hir::Node::Expr(&hir::Expr {
-                    kind: hir::ExprKind::Closure(hir::Closure { .. }),
+                    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,
@@ -2023,13 +2024,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
 
         if let (Some(expr), Some(_), Some(fn_decl)) = (expression, blk_id, parent_item.fn_decl()) {
             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, parent_id,
             );
         }
 

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

@@ -942,7 +942,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     pub(in super::super) fn get_node_fn_decl(
         &self,
         node: Node<'tcx>,
-    ) -> Option<(hir::HirId, &'tcx hir::FnDecl<'tcx>, Ident, bool)> {
+    ) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, Ident, bool)> {
         match node {
             Node::Item(&hir::Item {
                 ident,
@@ -953,25 +953,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 // 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((
-                    hir::HirId::make_owner(owner_id.def_id),
-                    &sig.decl,
-                    ident,
-                    ident.name != sym::main,
-                ))
+                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((hir::HirId::make_owner(owner_id.def_id), &sig.decl, ident, true)),
+            }) => Some((owner_id.def_id, &sig.decl, ident, true)),
             Node::ImplItem(&hir::ImplItem {
                 ident,
                 kind: hir::ImplItemKind::Fn(ref sig, ..),
                 owner_id,
                 ..
-            }) => Some((hir::HirId::make_owner(owner_id.def_id), &sig.decl, ident, false)),
+            }) => Some((owner_id.def_id, &sig.decl, ident, false)),
             Node::Expr(&hir::Expr {
                 hir_id,
                 kind:
@@ -1001,12 +996,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     }) => (ident, sig, owner_id),
                     _ => return None,
                 };
-                Some((
-                    hir::HirId::make_owner(owner_id.def_id),
-                    &sig.decl,
-                    ident,
-                    ident.name != sym::main,
-                ))
+                Some((owner_id.def_id, &sig.decl, ident, ident.name != sym::main))
             }
             _ => None,
         }
@@ -1017,7 +1007,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     pub fn get_fn_decl(
         &self,
         blk_id: hir::HirId,
-    ) -> Option<(hir::HirId, &'tcx hir::FnDecl<'tcx>, bool)> {
+    ) -> 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_return_block(blk_id).and_then(|blk_id| {

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -9,6 +9,7 @@ use crate::method::probe::{IsSuggestion, Mode, ProbeScope};
 use crate::rustc_middle::ty::Article;
 use core::cmp::min;
 use core::iter;
+use hir::def_id::LocalDefId;
 use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX};
 use rustc_data_structures::packed::Pu128;
 use rustc_errors::{Applicability, Diag, MultiSpan};
@@ -796,7 +797,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expected: Ty<'tcx>,
         found: Ty<'tcx>,
         can_suggest: bool,
-        fn_id: hir::HirId,
+        fn_id: LocalDefId,
     ) -> bool {
         let found =
             self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(found));
@@ -923,7 +924,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         err: &mut Diag<'_>,
         expected: Ty<'tcx>,
         found: Ty<'tcx>,
-        fn_id: hir::HirId,
+        fn_id: LocalDefId,
     ) {
         // Only apply the suggestion if:
         //  - the return type is a generic parameter
@@ -937,7 +938,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         let ty::Param(expected_ty_as_param) = expected.kind() else { return };
 
-        let fn_node = self.tcx.hir_node(fn_id);
+        let fn_node = self.tcx.hir_node_by_def_id(fn_id);
 
         let hir::Node::Item(hir::Item {
             kind:
@@ -1031,7 +1032,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expected: Ty<'tcx>,
         found: Ty<'tcx>,
         id: hir::HirId,
-        fn_id: hir::HirId,
+        fn_id: LocalDefId,
     ) {
         if !expected.is_unit() {
             return;
@@ -1083,11 +1084,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let can_return = match fn_decl.output {
             hir::FnRetTy::Return(ty) => {
                 let ty = self.lowerer().lower_ty(ty);
-                let bound_vars = self.tcx.late_bound_vars(fn_id);
+                let bound_vars = self.tcx.late_bound_vars(self.tcx.local_def_id_to_hir_id(fn_id));
                 let ty = self
                     .tcx
                     .instantiate_bound_regions_with_erased(Binder::bind_with_vars(ty, bound_vars));
-                let ty = match self.tcx.asyncness(fn_id.owner) {
+                let ty = match self.tcx.asyncness(fn_id) {
                     ty::Asyncness::Yes => self.get_impl_future_output_ty(ty).unwrap_or_else(|| {
                         span_bug!(
                             fn_decl.output.span(),
@@ -1108,8 +1109,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             _ => false,
         };
         if can_return
-            && let Some(owner_node) = self.tcx.hir_node(fn_id).as_owner()
-            && let Some(span) = expr.span.find_ancestor_inside(*owner_node.span())
+            && let Some(span) = expr.span.find_ancestor_inside(
+                self.tcx.hir().span_with_body(self.tcx.local_def_id_to_hir_id(fn_id)),
+            )
         {
             err.multipart_suggestion(
                 "you might have meant to return this value",

tests/ui/async-await/dont-ice-for-type-mismatch-in-closure-in-async.rs

@@ -0,0 +1,17 @@
+//@ edition: 2021
+
+fn call(_: impl Fn() -> bool) {}
+
+async fn test() {
+    call(|| -> Option<()> {
+        //~^ ERROR expected
+        if true {
+            false
+            //~^ ERROR mismatched types
+        }
+        true
+        //~^ ERROR mismatched types
+    })
+}
+
+fn main() {}

tests/ui/async-await/dont-ice-for-type-mismatch-in-closure-in-async.stderr

@@ -0,0 +1,46 @@
+error[E0308]: mismatched types
+  --> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:9:13
+   |
+LL | /         if true {
+LL | |             false
+   | |             ^^^^^ expected `()`, found `bool`
+LL | |
+LL | |         }
+   | |_________- expected this to be `()`
+
+error[E0308]: mismatched types
+  --> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:12:9
+   |
+LL |         true
+   |         ^^^^ expected `Option<()>`, found `bool`
+   |
+   = note: expected enum `Option<()>`
+              found type `bool`
+
+error[E0271]: expected `{[email protected]:6:10}` to be a closure that returns `bool`, but it returns `Option<()>`
+  --> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:6:10
+   |
+LL |       call(|| -> Option<()> {
+   |  _____----_^
+   | |     |
+   | |     required by a bound introduced by this call
+LL | |
+LL | |         if true {
+LL | |             false
+...  |
+LL | |
+LL | |     })
+   | |_____^ expected `bool`, found `Option<()>`
+   |
+   = note: expected type `bool`
+              found enum `Option<()>`
+note: required by a bound in `call`
+  --> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:3:25
+   |
+LL | fn call(_: impl Fn() -> bool) {}
+   |                         ^^^^ required by this bound in `call`
+
+error: aborting due to 3 previous errors
+
+Some errors have detailed explanations: E0271, E0308.
+For more information about an error, try `rustc --explain E0271`.