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 comes from. So here I want to take baby steps to see the impact of each step.

r? `@ghost`

Author: bors

compiler/rustc_middle/src/mir/mod.rs

@@ -310,6 +310,13 @@ impl<'tcx> CoroutineInfo<'tcx> {
     }
 }
 
+/// Some item that needs to monomorphize successfully for a MIR body to be considered well-formed.
+#[derive(Copy, Clone, TyEncodable, TyDecodable, Debug, HashStable, 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> {
@@ -375,6 +382,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.
@@ -445,6 +455,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,
@@ -473,6 +484,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/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,22 @@
 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};
 
 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 +29,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

@@ -734,6 +734,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);
 
@@ -915,6 +926,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>,

tests/ui/consts/const-eval/erroneous-const.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/erroneous-const2.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.stderr

@@ -1,20 +1,23 @@
+//@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.
     if false {
-        let _ = PrintName::<T>::VOID;
+        let _ = Fail::<T>::C;
     }
 }
+
 pub fn main() {
-    no_codegen::<i32>();
+    called::<i32>();
 }

tests/ui/consts/monomorphization/dead-code-in-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:16: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/monomorphization/dead-code-in-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:16: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/unused-broken-const-late.rs tests/ui/consts/monomorphization/dead-code-in-called-fn.rs

@@ -1,16 +1,19 @@
+//@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
+        Fail::<T>::C; //~ constant
     }
 }
 

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

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

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

@@ -0,0 +1,13 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/dead-code-in-dead-fn.rs:9:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/dead-code-in-dead-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 not_called::<i32>`
+
+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/monomorphization/dead-code-in-const-called-fn.rs

@@ -0,0 +1,28 @@
+//@revisions: opt no-opt
+//@ build-fail
+//@[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 Fail<T>(T);
+impl<T> Fail<T> {
+    const C: () = panic!(); //~ERROR evaluation of `Fail::<i32>::C` failed
+}
+
+// This function is not actually called, but it is mentioned in a function that is called.
+// Make sure we still find this error.
+#[inline(never)]
+fn not_called<T>() {
+    let _ = Fail::<T>::C;
+}
+
+#[inline(never)]
+fn called<T>() {
+    if false {
+        not_called::<T>();
+    }
+}
+
+pub fn main() {
+    called::<i32>();
+}