Rollup merge of #91410 - ecstatic-morse:const-precise-live-drops-take-2, r=oli-obk

Move `#![feature(const_precise_live_drops)]` checks earlier in the pipeline

Should mitigate the issues found during MCP on #73255.

Once this is done, we should clean up the queries a bit, since I think `mir_drops_elaborated_and_const_checked` can be merged back into `mir_promoted`.

Fixes #90770.

cc ``@rust-lang/wg-const-eval``
r? ``@nikomatsakis`` (since they reviewed #71824)

compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs

@@ -80,7 +80,8 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
         trace!("visit_terminator: terminator={:?} location={:?}", terminator, location);
 
         match &terminator.kind {
-            mir::TerminatorKind::Drop { place: dropped_place, .. } => {
+            mir::TerminatorKind::Drop { place: dropped_place, .. }
+            | mir::TerminatorKind::DropAndReplace { place: dropped_place, .. } => {
                 let dropped_ty = dropped_place.ty(self.body, self.tcx).ty;
                 if !NeedsNonConstDrop::in_any_value_of_ty(self.ccx, dropped_ty) {
                     // Instead of throwing a bug, we just return here. This is because we have to
@@ -104,11 +105,6 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
                 }
             }
 
-            mir::TerminatorKind::DropAndReplace { .. } => span_bug!(
-                terminator.source_info.span,
-                "`DropAndReplace` should be removed by drop elaboration",
-            ),
-
             mir::TerminatorKind::Abort
             | mir::TerminatorKind::Call { .. }
             | mir::TerminatorKind::Assert { .. }

compiler/rustc_middle/src/mir/mod.rs

@@ -1803,6 +1803,16 @@ impl<V, T> ProjectionElem<V, T> {
             | Self::Downcast(_, _) => false,
         }
     }
+
+    /// Returns `true` if this is a `Downcast` projection with the given `VariantIdx`.
+    pub fn is_downcast_to(&self, v: VariantIdx) -> bool {
+        matches!(*self, Self::Downcast(_, x) if x == v)
+    }
+
+    /// Returns `true` if this is a `Field` projection with the given index.
+    pub fn is_field_to(&self, f: Field) -> bool {
+        matches!(*self, Self::Field(x, _) if x == f)
+    }
 }
 
 /// Alias for projections as they appear in places, where the base is a place

compiler/rustc_mir_transform/src/lib.rs

@@ -60,8 +60,10 @@ mod match_branches;
 mod multiple_return_terminators;
 mod normalize_array_len;
 mod nrvo;
+mod remove_false_edges;
 mod remove_noop_landing_pads;
 mod remove_storage_markers;
+mod remove_uninit_drops;
 mod remove_unneeded_drops;
 mod remove_zsts;
 mod required_consts;
@@ -75,7 +77,7 @@ mod simplify_try;
 mod uninhabited_enum_branching;
 mod unreachable_prop;
 
-use rustc_const_eval::transform::check_consts;
+use rustc_const_eval::transform::check_consts::{self, ConstCx};
 use rustc_const_eval::transform::promote_consts;
 use rustc_const_eval::transform::validate;
 use rustc_mir_dataflow::rustc_peek;
@@ -444,8 +446,20 @@ fn mir_drops_elaborated_and_const_checked<'tcx>(
     let (body, _) = tcx.mir_promoted(def);
     let mut body = body.steal();
 
+    // IMPORTANT
+    remove_false_edges::RemoveFalseEdges.run_pass(tcx, &mut body);
+
+    // Do a little drop elaboration before const-checking if `const_precise_live_drops` is enabled.
+    //
+    // FIXME: Can't use `run_passes` for these, since `run_passes` SILENTLY DOES NOTHING IF THE MIR
+    // PHASE DOESN'T CHANGE.
+    if check_consts::post_drop_elaboration::checking_enabled(&ConstCx::new(tcx, &body)) {
+        simplify::SimplifyCfg::new("remove-false-edges").run_pass(tcx, &mut body);
+        remove_uninit_drops::RemoveUninitDrops.run_pass(tcx, &mut body);
+        check_consts::post_drop_elaboration::check_live_drops(tcx, &body);
+    }
+
     run_post_borrowck_cleanup_passes(tcx, &mut body);
-    check_consts::post_drop_elaboration::check_live_drops(tcx, &body);
     tcx.alloc_steal_mir(body)
 }
 
@@ -455,7 +469,7 @@ fn run_post_borrowck_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tc
 
     let post_borrowck_cleanup: &[&dyn MirPass<'tcx>] = &[
         // Remove all things only needed by analysis
-        &simplify_branches::SimplifyBranches::new("initial"),
+        &simplify_branches::SimplifyConstCondition::new("initial"),
         &remove_noop_landing_pads::RemoveNoopLandingPads,
         &cleanup_post_borrowck::CleanupNonCodegenStatements,
         &simplify::SimplifyCfg::new("early-opt"),
@@ -514,13 +528,13 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
         &instcombine::InstCombine,
         &separate_const_switch::SeparateConstSwitch,
         &const_prop::ConstProp,
-        &simplify_branches::SimplifyBranches::new("after-const-prop"),
+        &simplify_branches::SimplifyConstCondition::new("after-const-prop"),
         &early_otherwise_branch::EarlyOtherwiseBranch,
         &simplify_comparison_integral::SimplifyComparisonIntegral,
         &simplify_try::SimplifyArmIdentity,
         &simplify_try::SimplifyBranchSame,
         &dest_prop::DestinationPropagation,
-        &simplify_branches::SimplifyBranches::new("final"),
+        &simplify_branches::SimplifyConstCondition::new("final"),
         &remove_noop_landing_pads::RemoveNoopLandingPads,
         &simplify::SimplifyCfg::new("final"),
         &nrvo::RenameReturnPlace,

compiler/rustc_mir_transform/src/remove_false_edges.rs

@@ -0,0 +1,29 @@
+use rustc_middle::mir::{Body, TerminatorKind};
+use rustc_middle::ty::TyCtxt;
+
+use crate::MirPass;
+
+/// Removes `FalseEdge` and `FalseUnwind` terminators from the MIR.
+///
+/// These are only needed for borrow checking, and can be removed afterwards.
+///
+/// FIXME: This should probably have its own MIR phase.
+pub struct RemoveFalseEdges;
+
+impl<'tcx> MirPass<'tcx> for RemoveFalseEdges {
+    fn run_pass(&self, _: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
+        for block in body.basic_blocks_mut() {
+            let terminator = block.terminator_mut();
+            terminator.kind = match terminator.kind {
+                TerminatorKind::FalseEdge { real_target, .. } => {
+                    TerminatorKind::Goto { target: real_target }
+                }
+                TerminatorKind::FalseUnwind { real_target, .. } => {
+                    TerminatorKind::Goto { target: real_target }
+                }
+
+                _ => continue,
+            }
+        }
+    }
+}

compiler/rustc_mir_transform/src/remove_uninit_drops.rs

@@ -0,0 +1,171 @@
+use rustc_index::bit_set::BitSet;
+use rustc_middle::mir::{Body, Field, Rvalue, Statement, StatementKind, TerminatorKind};
+use rustc_middle::ty::subst::SubstsRef;
+use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt, VariantDef};
+use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
+use rustc_mir_dataflow::move_paths::{LookupResult, MoveData, MovePathIndex};
+use rustc_mir_dataflow::{self, move_path_children_matching, Analysis, MoveDataParamEnv};
+
+use crate::MirPass;
+
+/// Removes `Drop` and `DropAndReplace` terminators whose target is known to be uninitialized at
+/// that point.
+///
+/// This is redundant with drop elaboration, but we need to do it prior to const-checking, and
+/// running const-checking after drop elaboration makes it opimization dependent, causing issues
+/// like [#90770].
+///
+/// [#90770]: https://github.com/rust-lang/rust/issues/90770
+pub struct RemoveUninitDrops;
+
+impl<'tcx> MirPass<'tcx> for RemoveUninitDrops {
+    fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
+        let param_env = tcx.param_env(body.source.def_id());
+        let Ok(move_data) = MoveData::gather_moves(body, tcx, param_env) else {
+            // We could continue if there are move errors, but there's not much point since our
+            // init data isn't complete.
+            return;
+        };
+
+        let mdpe = MoveDataParamEnv { move_data, param_env };
+        let mut maybe_inits = MaybeInitializedPlaces::new(tcx, body, &mdpe)
+            .into_engine(tcx, body)
+            .pass_name("remove_uninit_drops")
+            .iterate_to_fixpoint()
+            .into_results_cursor(body);
+
+        let mut to_remove = vec![];
+        for (bb, block) in body.basic_blocks().iter_enumerated() {
+            let terminator = block.terminator();
+            let (TerminatorKind::Drop { place, .. } | TerminatorKind::DropAndReplace { place, .. })
+                = &terminator.kind
+            else { continue };
+
+            maybe_inits.seek_before_primary_effect(body.terminator_loc(bb));
+
+            // If there's no move path for the dropped place, it's probably a `Deref`. Let it alone.
+            let LookupResult::Exact(mpi) = mdpe.move_data.rev_lookup.find(place.as_ref()) else {
+                continue;
+            };
+
+            let should_keep = is_needs_drop_and_init(
+                tcx,
+                param_env,
+                maybe_inits.get(),
+                &mdpe.move_data,
+                place.ty(body, tcx).ty,
+                mpi,
+            );
+            if !should_keep {
+                to_remove.push(bb)
+            }
+        }
+
+        for bb in to_remove {
+            let block = &mut body.basic_blocks_mut()[bb];
+
+            let (TerminatorKind::Drop { target, .. } | TerminatorKind::DropAndReplace { target, .. })
+                = &block.terminator().kind
+            else { unreachable!() };
+
+            // Replace block terminator with `Goto`.
+            let target = *target;
+            let old_terminator_kind = std::mem::replace(
+                &mut block.terminator_mut().kind,
+                TerminatorKind::Goto { target },
+            );
+
+            // If this is a `DropAndReplace`, we need to emulate the assignment to the return place.
+            if let TerminatorKind::DropAndReplace { place, value, .. } = old_terminator_kind {
+                block.statements.push(Statement {
+                    source_info: block.terminator().source_info,
+                    kind: StatementKind::Assign(Box::new((place, Rvalue::Use(value)))),
+                });
+            }
+        }
+    }
+}
+
+fn is_needs_drop_and_init(
+    tcx: TyCtxt<'tcx>,
+    param_env: ParamEnv<'tcx>,
+    maybe_inits: &BitSet<MovePathIndex>,
+    move_data: &MoveData<'tcx>,
+    ty: Ty<'tcx>,
+    mpi: MovePathIndex,
+) -> bool {
+    // No need to look deeper if the root is definitely uninit or if it has no `Drop` impl.
+    if !maybe_inits.contains(mpi) || !ty.needs_drop(tcx, param_env) {
+        return false;
+    }
+
+    let field_needs_drop_and_init = |(f, f_ty, mpi)| {
+        let child = move_path_children_matching(move_data, mpi, |x| x.is_field_to(f));
+        let Some(mpi) = child else {
+            return f_ty.needs_drop(tcx, param_env);
+        };
+
+        is_needs_drop_and_init(tcx, param_env, maybe_inits, move_data, f_ty, mpi)
+    };
+
+    // This pass is only needed for const-checking, so it doesn't handle as many cases as
+    // `DropCtxt::open_drop`, since they aren't relevant in a const-context.
+    match ty.kind() {
+        ty::Adt(adt, substs) => {
+            let dont_elaborate = adt.is_union() || adt.is_manually_drop() || adt.has_dtor(tcx);
+            if dont_elaborate {
+                return true;
+            }
+
+            // Look at all our fields, or if we are an enum all our variants and their fields.
+            //
+            // If a field's projection *is not* present in `MoveData`, it has the same
+            // initializedness as its parent (maybe init).
+            //
+            // If its projection *is* present in `MoveData`, then the field may have been moved
+            // from separate from its parent. Recurse.
+            adt.variants.iter_enumerated().any(|(vid, variant)| {
+                // Enums have multiple variants, which are discriminated with a `Downcast` projection.
+                // Structs have a single variant, and don't use a `Downcast` projection.
+                let mpi = if adt.is_enum() {
+                    let downcast =
+                        move_path_children_matching(move_data, mpi, |x| x.is_downcast_to(vid));
+                    let Some(dc_mpi) = downcast else {
+                        return variant_needs_drop(tcx, param_env, substs, variant);
+                    };
+
+                    dc_mpi
+                } else {
+                    mpi
+                };
+
+                variant
+                    .fields
+                    .iter()
+                    .enumerate()
+                    .map(|(f, field)| (Field::from_usize(f), field.ty(tcx, substs), mpi))
+                    .any(field_needs_drop_and_init)
+            })
+        }
+
+        ty::Tuple(_) => ty
+            .tuple_fields()
+            .enumerate()
+            .map(|(f, f_ty)| (Field::from_usize(f), f_ty, mpi))
+            .any(field_needs_drop_and_init),
+
+        _ => true,
+    }
+}
+
+fn variant_needs_drop(
+    tcx: TyCtxt<'tcx>,
+    param_env: ParamEnv<'tcx>,
+    substs: SubstsRef<'tcx>,
+    variant: &VariantDef,
+) -> bool {
+    variant.fields.iter().any(|field| {
+        let f_ty = field.ty(tcx, substs);
+        f_ty.needs_drop(tcx, param_env)
+    })
+}

compiler/rustc_mir_transform/src/remove_unneeded_drops.rs

@@ -1,4 +1,8 @@
-//! This pass replaces a drop of a type that does not need dropping, with a goto
+//! This pass replaces a drop of a type that does not need dropping, with a goto.
+//!
+//! When the MIR is built, we check `needs_drop` before emitting a `Drop` for a place. This pass is
+//! useful because (unlike MIR building) it runs after type checking, so it can make use of
+//! `Reveal::All` to provide more precies type information.
 
 use crate::MirPass;
 use rustc_middle::mir::*;

compiler/rustc_mir_transform/src/simplify_branches.rs

@@ -1,22 +1,21 @@
-//! A pass that simplifies branches when their condition is known.
-
 use crate::MirPass;
 use rustc_middle::mir::*;
 use rustc_middle::ty::TyCtxt;
 
 use std::borrow::Cow;
 
-pub struct SimplifyBranches {
+/// A pass that replaces a branch with a goto when its condition is known.
+pub struct SimplifyConstCondition {
     label: String,
 }
 
-impl SimplifyBranches {
+impl SimplifyConstCondition {
     pub fn new(label: &str) -> Self {
-        SimplifyBranches { label: format!("SimplifyBranches-{}", label) }
+        SimplifyConstCondition { label: format!("SimplifyConstCondition-{}", label) }
     }
 }
 
-impl<'tcx> MirPass<'tcx> for SimplifyBranches {
+impl<'tcx> MirPass<'tcx> for SimplifyConstCondition {
     fn name(&self) -> Cow<'_, str> {
         Cow::Borrowed(&self.label)
     }
@@ -53,12 +52,6 @@ impl<'tcx> MirPass<'tcx> for SimplifyBranches {
                     Some(v) if v == expected => TerminatorKind::Goto { target },
                     _ => continue,
                 },
-                TerminatorKind::FalseEdge { real_target, .. } => {
-                    TerminatorKind::Goto { target: real_target }
-                }
-                TerminatorKind::FalseUnwind { real_target, .. } => {
-                    TerminatorKind::Goto { target: real_target }
-                }
                 _ => continue,
             };
         }

src/test/mir-opt/const_prop/switch_int.main.SimplifyConstCondition-after-const-prop.diff

@@ -1,5 +1,5 @@
-- // MIR for `main` before SimplifyBranches-after-const-prop
+- // MIR for `main` before SimplifyConstCondition-after-const-prop
-+ // MIR for `main` after SimplifyBranches-after-const-prop
++ // MIR for `main` after SimplifyConstCondition-after-const-prop
   
   fn main() -> () {
       let mut _0: ();                      // return place in scope 0 at $DIR/switch_int.rs:6:11: 6:11

src/test/mir-opt/const_prop/switch_int.rs

@@ -2,7 +2,7 @@
 fn foo(_: i32) { }
 
 // EMIT_MIR switch_int.main.ConstProp.diff
-// EMIT_MIR switch_int.main.SimplifyBranches-after-const-prop.diff
+// EMIT_MIR switch_int.main.SimplifyConstCondition-after-const-prop.diff
 fn main() {
     match 1 {
         1 => foo(0),

src/test/mir-opt/early_otherwise_branch_68867.rs

@@ -11,7 +11,7 @@ pub enum ViewportPercentageLength {
 }
 
 // EMIT_MIR early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.diff
-// EMIT_MIR early_otherwise_branch_68867.try_sum EarlyOtherwiseBranch.before SimplifyBranches-final.after
+// EMIT_MIR early_otherwise_branch_68867.try_sum EarlyOtherwiseBranch.before SimplifyConstCondition-final.after
 #[no_mangle]
 pub extern "C" fn try_sum(
     x: &ViewportPercentageLength,

src/test/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.before-SimplifyConstCondition-final.after.diff

@@ -1,5 +1,5 @@
 - // MIR for `try_sum` before EarlyOtherwiseBranch
-+ // MIR for `try_sum` after SimplifyBranches-final
++ // MIR for `try_sum` after SimplifyConstCondition-final
   
   fn try_sum(_1: &ViewportPercentageLength, _2: &ViewportPercentageLength) -> Result<ViewportPercentageLength, ()> {
       debug x => _1;                       // in scope 0 at $DIR/early_otherwise_branch_68867.rs:17:5: 17:6

src/test/mir-opt/simplify_if.main.SimplifyConstCondition-after-const-prop.diff

@@ -1,5 +1,5 @@
-- // MIR for `main` before SimplifyBranches-after-const-prop
+- // MIR for `main` before SimplifyConstCondition-after-const-prop
-+ // MIR for `main` after SimplifyBranches-after-const-prop
++ // MIR for `main` after SimplifyConstCondition-after-const-prop
   
   fn main() -> () {
       let mut _0: ();                      // return place in scope 0 at $DIR/simplify_if.rs:5:11: 5:11

src/test/mir-opt/simplify_if.rs

@@ -1,7 +1,7 @@
 #[inline(never)]
 fn noop() {}
 
-// EMIT_MIR simplify_if.main.SimplifyBranches-after-const-prop.diff
+// EMIT_MIR simplify_if.main.SimplifyConstCondition-after-const-prop.diff
 fn main() {
     if false {
         noop();

src/test/ui/consts/drop_zst.rs

@@ -0,0 +1,17 @@
+// check-fail
+
+#![feature(const_precise_live_drops)]
+
+struct S;
+
+impl Drop for S {
+    fn drop(&mut self) {
+        println!("Hello!");
+    }
+}
+
+const fn foo() {
+    let s = S; //~ destructor
+}
+
+fn main() {}

src/test/ui/consts/drop_zst.stderr

@@ -0,0 +1,9 @@
+error[E0493]: destructors cannot be evaluated at compile-time
+  --> $DIR/drop_zst.rs:14:9
+   |
+LL |     let s = S;
+   |         ^ constant functions cannot evaluate destructors
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0493`.