Auto merge of #122258 - RalfJung:required-fns, r=<try>

Draft: monomorphize things from dead code, too

This is another attempt at fixing #107503. The previous attempt at #112879 seems stuck in figuring out where the [perf regression](https://perf.rust-lang.org/compare.html?start=c55d1ee8d4e3162187214692229a63c2cc5e0f31&end=ec8de1ebe0d698b109beeaaac83e60f4ef8bb7d1&stat=instructions:u) comes from. So here I want to take baby steps to see the impact of each step.

r? `@ghost`

compiler/rustc_middle/src/mir/mod.rs

@@ -312,6 +312,14 @@ impl<'tcx> CoroutineInfo<'tcx> {
     }
 }
 
+/// Some item that needs to monomorphize successfully for a MIR body to be considered well-formed.
+#[derive(Copy, Clone, PartialEq, Debug, HashStable, TyEncodable, TyDecodable)]
+#[derive(TypeFoldable, TypeVisitable)]
+pub enum RequiredItem<'tcx> {
+    Fn(DefId, GenericArgsRef<'tcx>),
+    Drop(Ty<'tcx>),
+}
+
 /// The lowered representation of a single function.
 #[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable, TypeFoldable, TypeVisitable)]
 pub struct Body<'tcx> {
@@ -377,6 +385,9 @@ pub struct Body<'tcx> {
     /// We hold in this field all the constants we are not able to evaluate yet.
     pub required_consts: Vec<ConstOperand<'tcx>>,
 
+    /// Further items that need to monomorphize successfully for this MIR to be well-formed.
+    pub required_items: Vec<RequiredItem<'tcx>>,
+
     /// Does this body use generic parameters. This is used for the `ConstEvaluatable` check.
     ///
     /// Note that this does not actually mean that this body is not computable right now.
@@ -447,6 +458,7 @@ impl<'tcx> Body<'tcx> {
             var_debug_info,
             span,
             required_consts: Vec::new(),
+            required_items: Vec::new(),
             is_polymorphic: false,
             injection_phase: None,
             tainted_by_errors,
@@ -475,6 +487,7 @@ impl<'tcx> Body<'tcx> {
             spread_arg: None,
             span: DUMMY_SP,
             required_consts: Vec::new(),
+            required_items: Vec::new(),
             var_debug_info: Vec::new(),
             is_polymorphic: false,
             injection_phase: None,

compiler/rustc_mir_build/src/build/custom/mod.rs

@@ -56,6 +56,7 @@ pub(super) fn build_custom_mir<'tcx>(
         var_debug_info: Vec::new(),
         span,
         required_consts: Vec::new(),
+        required_items: Vec::new(),
         is_polymorphic: false,
         tainted_by_errors: None,
         injection_phase: None,

compiler/rustc_mir_transform/src/inline.rs

@@ -565,7 +565,8 @@ impl<'tcx> Inliner<'tcx> {
         mut callee_body: Body<'tcx>,
     ) {
         let terminator = caller_body[callsite.block].terminator.take().unwrap();
-        let TerminatorKind::Call { args, destination, unwind, target, .. } = terminator.kind else {
+        let TerminatorKind::Call { func, args, destination, unwind, target, .. } = terminator.kind
+        else {
             bug!("unexpected terminator kind {:?}", terminator.kind);
         };
 
@@ -717,6 +718,18 @@ impl<'tcx> Inliner<'tcx> {
                 Const::Val(..) | Const::Unevaluated(..) => true,
             },
         ));
+        //  Now that we incorporated the callee's `required_consts`, we can remove the callee from
+        //  `required_items` -- but we have to take their `required_consts` in return.
+        let callee_item = {
+            // We need to reconstruct the `required_item` for the callee.
+            let func_ty = func.ty(caller_body, self.tcx);
+            match func_ty.kind() {
+                ty::FnDef(def_id, args) => RequiredItem::Fn(*def_id, args),
+                _ => bug!(),
+            }
+        };
+        caller_body.required_items.retain(|item| *item != callee_item);
+        caller_body.required_items.extend(callee_body.required_items);
     }
 
     fn make_call_args(

compiler/rustc_mir_transform/src/lib.rs

@@ -344,11 +344,14 @@ fn mir_promoted(
     }
 
     let mut required_consts = Vec::new();
-    let mut required_consts_visitor = RequiredConstsVisitor::new(&mut required_consts);
+    let mut required_items = Vec::new();
+    let mut required_consts_visitor =
+        RequiredConstsVisitor::new(tcx, &body, &mut required_consts, &mut required_items);
     for (bb, bb_data) in traversal::reverse_postorder(&body) {
         required_consts_visitor.visit_basic_block_data(bb, bb_data);
     }
     body.required_consts = required_consts;
+    body.required_items = required_items;
 
     // What we need to run borrowck etc.
     let promote_pass = promote_consts::PromoteTemps::default();

compiler/rustc_mir_transform/src/required_consts.rs

@@ -1,14 +1,23 @@
 use rustc_middle::mir::visit::Visitor;
-use rustc_middle::mir::{Const, ConstOperand, Location};
-use rustc_middle::ty::ConstKind;
+use rustc_middle::mir::{self, Const, ConstOperand, Location, RequiredItem};
+use rustc_middle::ty::{self, ConstKind, TyCtxt};
 
+#[allow(unused)]
 pub struct RequiredConstsVisitor<'a, 'tcx> {
+    tcx: TyCtxt<'tcx>,
+    body: &'a mir::Body<'tcx>,
     required_consts: &'a mut Vec<ConstOperand<'tcx>>,
+    required_items: &'a mut Vec<RequiredItem<'tcx>>,
 }
 
 impl<'a, 'tcx> RequiredConstsVisitor<'a, 'tcx> {
-    pub fn new(required_consts: &'a mut Vec<ConstOperand<'tcx>>) -> Self {
-        RequiredConstsVisitor { required_consts }
+    pub fn new(
+        tcx: TyCtxt<'tcx>,
+        body: &'a mir::Body<'tcx>,
+        required_consts: &'a mut Vec<ConstOperand<'tcx>>,
+        required_items: &'a mut Vec<RequiredItem<'tcx>>,
+    ) -> Self {
+        RequiredConstsVisitor { tcx, body, required_consts, required_items }
     }
 }
 
@@ -21,7 +30,27 @@ impl<'tcx> Visitor<'tcx> for RequiredConstsVisitor<'_, 'tcx> {
                 _ => bug!("only ConstKind::Param/Value should be encountered here, got {:#?}", c),
             },
             Const::Unevaluated(..) => self.required_consts.push(*constant),
-            Const::Val(..) => {}
+            Const::Val(_val, ty) => {
+                // This is how function items get referenced: via zero-sized constants of `FnDef` type
+                if let ty::FnDef(def_id, args) = ty.kind() {
+                    debug!("adding to required_items: {def_id:?}");
+                    self.required_items.push(RequiredItem::Fn(*def_id, args));
+                }
+            }
         }
     }
+
+    /*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_constant`. But we do need to handle `Drop`.
+            mir::TerminatorKind::Drop { place, .. } => {
+                let ty = place.ty(self.body, self.tcx).ty;
+                self.required_items.push(RequiredItem::Drop(ty));
+            }
+            _ => {}
+        }
+    }*/
 }

compiler/rustc_monomorphize/src/collector.rs

@@ -345,7 +345,7 @@ fn collect_items_rec<'tcx>(
         return;
     }
 
-    let mut used_items = Vec::new();
+    let mut used_items = MonoItems::new();
     let recursion_depth_reset;
 
     // Post-monomorphization errors MVP
@@ -738,6 +738,17 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
+    fn visit_body(&mut self, body: &mir::Body<'tcx>) {
+        for item in &body.required_items {
+            // All these also need monomorphization to ensure that if that leads to error, we find
+            // those errors.
+            let item = self.monomorphize(*item);
+            visit_required_item(self.tcx, item, self.output);
+        }
+
+        self.super_body(body);
+    }
+
     fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) {
         debug!("visiting rvalue {:?}", *rvalue);
 
@@ -919,6 +930,25 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
     }
 }
 
+fn visit_required_item<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    item: mir::RequiredItem<'tcx>,
+    output: &mut MonoItems<'tcx>,
+) {
+    let instance = match item {
+        mir::RequiredItem::Fn(def_id, args) => {
+            Instance::expect_resolve(tcx, ty::ParamEnv::reveal_all(), def_id, args)
+        }
+        mir::RequiredItem::Drop(ty) => Instance::resolve_drop_in_place(tcx, ty),
+    };
+    // We pretend this is a direct call, just to make sure this is visited at all.
+    // "indirect" would mean we also generate some shims, but we don't care about the
+    // generated code, just about the side-effect of code generation causing errors, so we
+    // can skip the shims.
+    // FIXME: track the span so that we can show it here.
+    visit_instance_use(tcx, instance, /*is_direct_call*/ true, DUMMY_SP, output);
+}
+
 fn visit_drop_use<'tcx>(
     tcx: TyCtxt<'tcx>,
     ty: Ty<'tcx>,
@@ -957,7 +987,7 @@ fn visit_instance_use<'tcx>(
     source: Span,
     output: &mut MonoItems<'tcx>,
 ) {
-    debug!("visit_item_use({:?}, is_direct_call={:?})", instance, is_direct_call);
+    debug!("visit_instance_use({:?}, is_direct_call={:?})", instance, is_direct_call);
     if !should_codegen_locally(tcx, &instance) {
         return;
     }

tests/ui/consts/required-consts/dead-code-in-called-fn.no-opt.stderr

@@ -0,0 +1,13 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/dead-code-in-called-fn.rs:9:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/dead-code-in-called-fn.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: the above error was encountered while instantiating `fn called::<i32>`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/required-consts/dead-code-in-called-fn.opt.stderr

@@ -0,0 +1,13 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/dead-code-in-called-fn.rs:9:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/dead-code-in-called-fn.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: the above error was encountered while instantiating `fn called::<i32>`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/const-eval/unused-broken-const-late.rs → tests/ui/consts/required-consts/dead-code-in-called-fn.rs

@@ -1,20 +1,24 @@
+//@revisions: opt no-opt
 //@ build-fail
-//@ compile-flags: -O
+//@[opt] compile-flags: -O
 //! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is
 //! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090)
 
-struct PrintName<T>(T);
-impl<T> PrintName<T> {
-    const VOID: () = panic!(); //~ERROR evaluation of `PrintName::<i32>::VOID` failed
+struct Fail<T>(T);
+impl<T> Fail<T> {
+    const C: () = panic!(); //~ERROR evaluation of `Fail::<i32>::C` failed
 }
 
-fn no_codegen<T>() {
+#[inline(never)]
+fn called<T>() {
     // Any function that is called is guaranteed to have all consts that syntactically
     // appear in its body evaluated, even if they only appear in dead code.
+    // This relies on mono-item collection checking `required_consts` in collected functions.
     if false {
-        let _ = PrintName::<T>::VOID;
+        let _ = Fail::<T>::C;
     }
 }
+
 pub fn main() {
-    no_codegen::<i32>();
+    called::<i32>();
 }

tests/ui/consts/required-consts/dead-code-in-const-called-fn.no-opt.stderr

@@ -0,0 +1,17 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/dead-code-in-const-called-fn.rs:8:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/dead-code-in-const-called-fn.rs:8: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/dead-code-in-const-called-fn.rs:17:9
+   |
+LL |         Fail::<T>::C;
+   |         ^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/required-consts/dead-code-in-const-called-fn.opt.stderr

@@ -0,0 +1,17 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/dead-code-in-const-called-fn.rs:8:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/dead-code-in-const-called-fn.rs:8: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/dead-code-in-const-called-fn.rs:17:9
+   |
+LL |         Fail::<T>::C;
+   |         ^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/const-eval/erroneous-const.rs → tests/ui/consts/required-consts/dead-code-in-const-called-fn.rs

@@ -1,16 +1,20 @@
+//@revisions: opt no-opt
+//@[opt] compile-flags: -O
 //! Make sure we error on erroneous consts even if they are unused.
 #![allow(unconditional_panic)]
 
-struct PrintName<T>(T);
-impl<T> PrintName<T> {
-    const VOID: () = [()][2]; //~ERROR evaluation of `PrintName::<i32>::VOID` failed
+struct Fail<T>(T);
+impl<T> Fail<T> {
+    const C: () = panic!(); //~ERROR evaluation of `Fail::<i32>::C` failed
 }
 
+#[inline(never)]
 const fn no_codegen<T>() {
     if false {
         // This bad constant is only used in dead code in a no-codegen function... and yet we still
         // must make sure that the build fails.
-            PrintName::<T>::VOID; //~ constant
+        // This relies on const-eval evaluating all `required_consts` of `const fn`.
+        Fail::<T>::C; //~ constant
     }
 }
 

tests/ui/consts/required-consts/dead-code-in-dead-drop.no-opt.stderr

@@ -0,0 +1,14 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/dead-code-in-dead-drop.rs:9:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/dead-code-in-dead-drop.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: the above error was encountered while instantiating `fn <Fail<i32> as std::ops::Drop>::drop`
+  --> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.