mentioned_items: record all callee types, whether they are FnDef or not

RalfJung · RalfJung · commit f7165996f37d · 2024-03-18T16:56:55.000+01:00
They may become FnDef during monomorphization!
diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs
@@ -318,13 +318,13 @@ impl<'tcx> CoroutineInfo<'tcx> {
 #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, HashStable, TyEncodable, TyDecodable)]
 #[derive(TypeFoldable, TypeVisitable)]
 pub enum MentionedItem<'tcx> {
-    Fn(DefId, GenericArgsRef<'tcx>),
+    /// A function that gets called. We don't necessarily know its precise type yet, since it can be
+    /// hidden behind a generic.
+    Fn(Ty<'tcx>),
+    /// A type that has its drop shim called.
     Drop(Ty<'tcx>),
     /// Unsizing casts might require vtables, so we have to record them.
-    UnsizeCast {
-        source_ty: Ty<'tcx>,
-        target_ty: Ty<'tcx>,
-    },
+    UnsizeCast { source_ty: Ty<'tcx>, target_ty: Ty<'tcx> },
     /// A closure that is coerced to a function pointer.
     Closure(DefId, GenericArgsRef<'tcx>),
 }
diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs
@@ -723,15 +723,9 @@ impl<'tcx> Inliner<'tcx> {
         // some extra work here to save the monomorphization collector work later. It helps a lot,
         // since monomorphization can avoid a lot of work when the "mentioned items" are similar to
         // the actually used items. By doing this we can entirely avoid visiting the callee!
-        let callee_item = {
-            // We need to reconstruct the `required_item` for the callee so that we can find and
-            // remove it.
-            let func_ty = func.ty(caller_body, self.tcx);
-            match func_ty.kind() {
-                ty::FnDef(def_id, args) => MentionedItem::Fn(*def_id, args),
-                _ => bug!(),
-            }
-        };
+        // We need to reconstruct the `required_item` for the callee so that we can find and
+        // remove it.
+        let callee_item = MentionedItem::Fn(func.ty(caller_body, self.tcx));
         if let Some(idx) =
             caller_body.mentioned_items.iter().position(|item| item.node == callee_item)
         {
diff --git a/compiler/rustc_mir_transform/src/mentioned_items.rs b/compiler/rustc_mir_transform/src/mentioned_items.rs
@@ -1,6 +1,5 @@
 use rustc_middle::mir::visit::Visitor;
 use rustc_middle::mir::{self, Location, MentionedItem, MirPass};
-use rustc_middle::ty::Ty;
 use rustc_middle::ty::{self, adjustment::PointerCoercion, TyCtxt};
 use rustc_session::Session;
 use rustc_span::source_map::Spanned;
@@ -30,33 +29,44 @@ impl<'tcx> MirPass<'tcx> for MentionedItems {
     }
 }
 
+// This visitor is carefully in sync with the one in `rustc_monomorphize::collector`. We are
+// visiting the exact same places but then instead of monomorphizing and creating `MonoItems`, we
+// have to remain generic and just recording the relevant information in `mentioned_items`, where it
+// will then be monomorphized later during "mentioned items" collection.
 impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> {
-    fn visit_ty(&mut self, ty: Ty<'tcx>, ty_context: mir::visit::TyContext) {
-        if let ty::FnDef(def_id, args) = ty.kind() {
-            debug!("adding to required_items: {def_id:?}");
-            self.mentioned_items.push(Spanned {
-                node: MentionedItem::Fn(*def_id, args),
-                span: self.body.span_for_ty_context(ty_context),
-            });
-        }
-    }
-
     fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) {
         self.super_terminator(terminator, location);
-        match terminator.kind {
-            // We don't need to handle `Call` as we already handled all function type operands in
-            // `visit_ty`. But we do need to handle `Drop`.
+        let span = || self.body.source_info(location).span;
+        match &terminator.kind {
+            mir::TerminatorKind::Call { func, .. } => {
+                let callee_ty = func.ty(self.body, self.tcx);
+                self.mentioned_items
+                    .push(Spanned { node: MentionedItem::Fn(callee_ty), span: span() });
+            }
             mir::TerminatorKind::Drop { place, .. } => {
                 let ty = place.ty(self.body, self.tcx).ty;
-                let span = self.body.source_info(location).span;
-                self.mentioned_items.push(Spanned { node: MentionedItem::Drop(ty), span });
+                self.mentioned_items.push(Spanned { node: MentionedItem::Drop(ty), span: span() });
+            }
+            mir::TerminatorKind::InlineAsm { ref operands, .. } => {
+                for op in operands {
+                    match *op {
+                        mir::InlineAsmOperand::SymFn { ref value } => {
+                            self.mentioned_items.push(Spanned {
+                                node: MentionedItem::Fn(value.const_.ty()),
+                                span: span(),
+                            });
+                        }
+                        _ => {}
+                    }
+                }
             }
             _ => {}
         }
     }
 
     fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) {
         self.super_rvalue(rvalue, location);
+        let span = || self.body.source_info(location).span;
         match *rvalue {
             // We need to detect unsizing casts that required vtables.
             mir::Rvalue::Cast(
@@ -65,13 +75,12 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> {
                 target_ty,
             )
             | mir::Rvalue::Cast(mir::CastKind::DynStar, ref operand, target_ty) => {
-                let span = self.body.source_info(location).span;
                 self.mentioned_items.push(Spanned {
                     node: MentionedItem::UnsizeCast {
                         source_ty: operand.ty(self.body, self.tcx),
                         target_ty,
                     },
-                    span,
+                    span: span(),
                 });
             }
             // Similarly, record closures that are turned into function pointers.
@@ -80,17 +89,26 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> {
                 ref operand,
                 _,
             ) => {
-                let span = self.body.source_info(location).span;
                 let source_ty = operand.ty(self.body, self.tcx);
                 match *source_ty.kind() {
                     ty::Closure(def_id, args) => {
-                        self.mentioned_items
-                            .push(Spanned { node: MentionedItem::Closure(def_id, args), span });
+                        self.mentioned_items.push(Spanned {
+                            node: MentionedItem::Closure(def_id, args),
+                            span: span(),
+                        });
                     }
                     _ => bug!(),
                 }
             }
-            // Function pointer casts are already handled by `visit_ty` above.
+            // And finally, function pointer reification casts.
+            mir::Rvalue::Cast(
+                mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer),
+                ref operand,
+                _,
+            ) => {
+                let fn_ty = operand.ty(self.body, self.tcx);
+                self.mentioned_items.push(Spanned { node: MentionedItem::Fn(fn_ty), span: span() });
+            }
             _ => {}
         }
     }
diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs
@@ -994,9 +994,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
             mir::TerminatorKind::Call { ref func, ref args, ref fn_span, .. } => {
                 let callee_ty = func.ty(self.body, tcx);
                 // *Before* monomorphizing, record that we already handled this mention.
-                if let ty::FnDef(def_id, args) = callee_ty.kind() {
-                    self.used_mentioned_items.insert(MentionedItem::Fn(*def_id, args));
-                }
+                self.used_mentioned_items.insert(MentionedItem::Fn(callee_ty));
                 let callee_ty = self.monomorphize(callee_ty);
                 self.check_fn_args_move_size(callee_ty, args, *fn_span, location);
                 visit_fn_use(self.tcx, callee_ty, true, source, &mut self.used_items)
@@ -1076,6 +1074,8 @@ fn visit_drop_use<'tcx>(
     visit_instance_use(tcx, instance, is_direct_call, source, output);
 }
 
+/// For every call of this function in the visitor, make sure there is a matching call in the
+/// `mentioned_items` pass!
 fn visit_fn_use<'tcx>(
     tcx: TyCtxt<'tcx>,
     ty: Ty<'tcx>,
@@ -1700,13 +1700,16 @@ fn visit_mentioned_item<'tcx>(
     output: &mut MonoItems<'tcx>,
 ) {
     match *item {
-        MentionedItem::Fn(def_id, args) => {
-            let instance = Instance::expect_resolve(tcx, ty::ParamEnv::reveal_all(), def_id, args);
-            // `visit_instance_use` was written for "used" item collection but works just as well
-            // for "mentioned" item collection.
-            // We can set `is_direct_call`; that just means we'll skip a bunch of shims that anyway
-            // can't have their own failing constants.
-            visit_instance_use(tcx, instance, /*is_direct_call*/ true, span, output);
+        MentionedItem::Fn(ty) => {
+            if let ty::FnDef(def_id, args) = *ty.kind() {
+                let instance =
+                    Instance::expect_resolve(tcx, ty::ParamEnv::reveal_all(), def_id, args);
+                // `visit_instance_use` was written for "used" item collection but works just as well
+                // for "mentioned" item collection.
+                // We can set `is_direct_call`; that just means we'll skip a bunch of shims that anyway
+                // can't have their own failing constants.
+                visit_instance_use(tcx, instance, /*is_direct_call*/ true, span, output);
+            }
         }
         MentionedItem::Drop(ty) => {
             visit_drop_use(tcx, ty, /*is_direct_call*/ true, span, output);
diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn-behind-assoc-type.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fn-behind-assoc-type.noopt.stderr
@@ -0,0 +1,20 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/collect-in-dead-fn-behind-assoc-type.rs:9:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn-behind-assoc-type.rs:9:19
+   |
+   = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: erroneous constant encountered
+  --> $DIR/collect-in-dead-fn-behind-assoc-type.rs:14:17
+   |
+LL |         let _ = Fail::<T>::C;
+   |                 ^^^^^^^^^^^^
+
+note: the above error was encountered while instantiating `fn not_called::<i32>`
+  --> $SRC_DIR/core/src/ops/function.rs:LL:COL
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.
diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn-behind-assoc-type.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fn-behind-assoc-type.opt.stderr
@@ -0,0 +1,20 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/collect-in-dead-fn-behind-assoc-type.rs:9:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn-behind-assoc-type.rs:9:19
+   |
+   = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: erroneous constant encountered
+  --> $DIR/collect-in-dead-fn-behind-assoc-type.rs:14:17
+   |
+LL |         let _ = Fail::<T>::C;
+   |                 ^^^^^^^^^^^^
+
+note: the above error was encountered while instantiating `fn not_called::<i32>`
+  --> $SRC_DIR/core/src/ops/function.rs:LL:COL
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.
diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn-behind-assoc-type.rs b/tests/ui/consts/required-consts/collect-in-dead-fn-behind-assoc-type.rs
@@ -0,0 +1,45 @@
+#![feature(impl_trait_in_assoc_type)]
+//@revisions: noopt opt
+//@ build-fail
+//@[opt] compile-flags: -O
+//! This fails without optimizations, so it should also fail with optimizations.
+
+struct Fail<T>(T);
+impl<T> Fail<T> {
+    const C: () = panic!(); //~ERROR evaluation of `Fail::<i32>::C` failed
+}
+
+fn not_called<T>() {
+    if false {
+        let _ = Fail::<T>::C;
+    }
+}
+
+fn callit_not(f: impl Fn()) {
+    if false {
+        f();
+    }
+}
+
+// Using `Fn` here is important; with `FnOnce` another shim gets involved which somehow makes this
+// easier to collect properly.
+trait Hideaway {
+    type T: Fn();
+    const C: Self::T;
+}
+impl Hideaway for () {
+    type T = impl Fn();
+    const C: Self::T = not_called::<i32>;
+}
+
+fn reveal<T: Hideaway>() {
+    if false {
+        callit_not(T::C);
+    }
+}
+
+fn main() {
+    if false {
+        reveal::<()>()
+    }
+}
diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn-behind-generic.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fn-behind-generic.opt.stderr
@@ -13,10 +13,7 @@ LL |         let _ = Fail::<T>::C;
    |                 ^^^^^^^^^^^^
 
 note: the above error was encountered while instantiating `fn not_called::<i32>`
-  --> $DIR/collect-in-dead-fn-behind-generic.rs:25:9
-   |
-LL |         callit_not(not_called::<i32>)
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  --> $SRC_DIR/core/src/ops/function.rs:LL:COL
 
 error: aborting due to 1 previous error
 
diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn-behind-opaque-type.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fn-behind-opaque-type.opt.stderr
@@ -13,10 +13,7 @@ LL |             let _ = Fail::<T>::C;
    |                     ^^^^^^^^^^^^
 
 note: the above error was encountered while instantiating `fn m::not_called::<i32>`
-  --> $DIR/collect-in-dead-fn-behind-opaque-type.rs:32:42
-   |
-LL |         let x: m::NotCalledFn = unsafe { std::mem::transmute(()) };
-   |                                          ^^^^^^^^^^^^^^^^^^^^^^^
+  --> $SRC_DIR/core/src/ops/function.rs:LL:COL
 
 error: aborting due to 1 previous error
 
