Disable inactive enum variant drop elaboration optimization

src/librustc_mir/borrow_check/mod.rs

@@ -204,6 +204,7 @@ fn do_mir_borrowck<'a, 'tcx>(
     let mdpe = MoveDataParamEnv { move_data, param_env };
 
     let mut flow_inits = MaybeInitializedPlaces::new(tcx, &body, &mdpe)
+        .mark_inactive_variants_as_uninit(true)
         .into_engine(tcx, &body, def.did.to_def_id())
         .iterate_to_fixpoint()
         .into_results_cursor(&body);

src/librustc_mir/dataflow/impls/mod.rs

@@ -67,21 +67,30 @@ pub use self::storage_liveness::{MaybeRequiresStorage, MaybeStorageLive};
 /// Similarly, at a given `drop` statement, the set-intersection
 /// between this data and `MaybeUninitializedPlaces` yields the set of
 /// places that would require a dynamic drop-flag at that statement.
-pub struct MaybeInitializedPlaces<'a, 'tcx> {
+pub struct MaybeInitializedPlaces<'a, 'tcx, M = &'a MoveDataParamEnv<'tcx>> {
     tcx: TyCtxt<'tcx>,
     body: &'a Body<'tcx>,
-    mdpe: &'a MoveDataParamEnv<'tcx>,
+    pub(crate) mdpe: M,
+    mark_inactive_variants_as_uninit: bool,
 }
 
-impl<'a, 'tcx> MaybeInitializedPlaces<'a, 'tcx> {
-    pub fn new(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, mdpe: &'a MoveDataParamEnv<'tcx>) -> Self {
-        MaybeInitializedPlaces { tcx, body, mdpe }
+impl<'a, 'tcx, M> MaybeInitializedPlaces<'a, 'tcx, M> {
+    pub fn new(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, mdpe: M) -> Self {
+        MaybeInitializedPlaces { tcx, body, mdpe, mark_inactive_variants_as_uninit: false }
+    }
+
+    pub fn mark_inactive_variants_as_uninit(mut self, yes: bool) -> Self {
+        self.mark_inactive_variants_as_uninit = yes;
+        self
     }
 }
 
-impl<'a, 'tcx> HasMoveData<'tcx> for MaybeInitializedPlaces<'a, 'tcx> {
+impl<'a, 'tcx, M> HasMoveData<'tcx> for MaybeInitializedPlaces<'a, 'tcx, M>
+where
+    M: std::borrow::Borrow<MoveDataParamEnv<'tcx>>,
+{
     fn move_data(&self) -> &MoveData<'tcx> {
-        &self.mdpe.move_data
+        &self.mdpe.borrow().move_data
     }
 }
 
@@ -138,8 +147,8 @@ impl<'a, 'tcx> MaybeUninitializedPlaces<'a, 'tcx> {
     ///
     /// This is correct in a vacuum but is not the default because it causes problems in the borrow
     /// checker, where this information gets propagated along `FakeEdge`s.
-    pub fn mark_inactive_variants_as_uninit(mut self) -> Self {
-        self.mark_inactive_variants_as_uninit = true;
+    pub fn mark_inactive_variants_as_uninit(mut self, yes: bool) -> Self {
+        self.mark_inactive_variants_as_uninit = yes;
         self
     }
 }
@@ -250,7 +259,7 @@ impl<'a, 'tcx> HasMoveData<'tcx> for EverInitializedPlaces<'a, 'tcx> {
     }
 }
 
-impl<'a, 'tcx> MaybeInitializedPlaces<'a, 'tcx> {
+impl<'a, 'tcx, M> MaybeInitializedPlaces<'a, 'tcx, M> {
     fn update_bits(
         trans: &mut impl GenKill<MovePathIndex>,
         path: MovePathIndex,
@@ -289,7 +298,10 @@ impl<'a, 'tcx> DefinitelyInitializedPlaces<'a, 'tcx> {
     }
 }
 
-impl<'tcx> AnalysisDomain<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
+impl<'tcx, M> AnalysisDomain<'tcx> for MaybeInitializedPlaces<'_, 'tcx, M>
+where
+    M: std::borrow::Borrow<MoveDataParamEnv<'tcx>>,
+{
     type Idx = MovePathIndex;
 
     const NAME: &'static str = "maybe_init";
@@ -299,7 +311,7 @@ impl<'tcx> AnalysisDomain<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
     }
 
     fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut BitSet<Self::Idx>) {
-        drop_flag_effects_for_function_entry(self.tcx, self.body, self.mdpe, |path, s| {
+        drop_flag_effects_for_function_entry(self.tcx, self.body, self.mdpe.borrow(), |path, s| {
             assert!(s == DropFlagState::Present);
             state.insert(path);
         });
@@ -310,16 +322,23 @@ impl<'tcx> AnalysisDomain<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
     }
 }
 
-impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
+impl<'tcx, M> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx, M>
+where
+    M: std::borrow::Borrow<MoveDataParamEnv<'tcx>>,
+{
     fn statement_effect(
         &self,
         trans: &mut impl GenKill<Self::Idx>,
         _statement: &mir::Statement<'tcx>,
         location: Location,
     ) {
-        drop_flag_effects_for_location(self.tcx, self.body, self.mdpe, location, |path, s| {
-            Self::update_bits(trans, path, s)
-        })
+        drop_flag_effects_for_location(
+            self.tcx,
+            self.body,
+            self.mdpe.borrow(),
+            location,
+            |path, s| Self::update_bits(trans, path, s),
+        )
     }
 
     fn terminator_effect(
@@ -328,9 +347,13 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
         _terminator: &mir::Terminator<'tcx>,
         location: Location,
     ) {
-        drop_flag_effects_for_location(self.tcx, self.body, self.mdpe, location, |path, s| {
-            Self::update_bits(trans, path, s)
-        })
+        drop_flag_effects_for_location(
+            self.tcx,
+            self.body,
+            self.mdpe.borrow(),
+            location,
+            |path, s| Self::update_bits(trans, path, s),
+        )
     }
 
     fn call_return_effect(
@@ -362,6 +385,10 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
         _adt: &ty::AdtDef,
         variant: VariantIdx,
     ) {
+        if !self.mark_inactive_variants_as_uninit {
+            return;
+        }
+
         // Kill all move paths that correspond to variants we know to be inactive along this
         // particular outgoing edge of a `SwitchInt`.
         drop_flag_effects::on_all_inactive_variants(
@@ -626,7 +653,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for EverInitializedPlaces<'_, 'tcx> {
     }
 }
 
-impl<'a, 'tcx> BottomValue for MaybeInitializedPlaces<'a, 'tcx> {
+impl<'a, 'tcx, M> BottomValue for MaybeInitializedPlaces<'a, 'tcx, M> {
     /// bottom = uninitialized
     const BOTTOM_VALUE: bool = false;
 }

src/librustc_mir/dataflow/move_paths/builder.rs

@@ -310,11 +310,10 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
             StatementKind::StorageDead(local) => {
                 self.gather_move(Place::from(*local));
             }
-            StatementKind::SetDiscriminant { .. } => {
-                span_bug!(
-                    stmt.source_info.span,
-                    "SetDiscriminant should not exist during borrowck"
-                );
+            StatementKind::SetDiscriminant { place, .. } => {
+                // `SetDiscriminant` should not exist during borrowck, but we need to handle
+                // it here to compute what places are moved from during later compilation stages.
+                self.create_move_path(**place);
             }
             StatementKind::Retag { .. }
             | StatementKind::AscribeUserType(..)

src/librustc_mir/dataflow/move_paths/mod.rs

@@ -307,6 +307,15 @@ pub enum LookupResult {
     Parent(Option<MovePathIndex>),
 }
 
+impl LookupResult {
+    pub fn expect_exact(self, s: &str) -> MovePathIndex {
+        match self {
+            Self::Exact(mpi) => mpi,
+            Self::Parent(_) => panic!("{}", s),
+        }
+    }
+}
+
 impl MovePathLookup {
     // Unlike the builder `fn move_path_for` below, this lookup
     // alternative will *not* create a MovePath on the fly for an

src/librustc_mir/transform/check_consts/post_drop_elaboration.rs

@@ -8,6 +8,9 @@ use super::ops;
 use super::qualifs::{NeedsDrop, Qualif};
 use super::validation::Qualifs;
 use super::ConstCx;
+use crate::dataflow::drop_flag_effects::on_all_drop_children_bits;
+use crate::dataflow::move_paths::MoveData;
+use crate::dataflow::{self, Analysis, MoveDataParamEnv, ResultsCursor};
 
 /// Returns `true` if we should use the more precise live drop checker that runs after drop
 /// elaboration.
@@ -31,14 +34,22 @@ pub fn check_live_drops(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &mir::Body<
 
     let ccx = ConstCx { body, tcx, def_id, const_kind, param_env: tcx.param_env(def_id) };
 
-    let mut visitor = CheckLiveDrops { ccx: &ccx, qualifs: Qualifs::default() };
+    let mut visitor = CheckLiveDrops { ccx: &ccx, qualifs: Qualifs::default(), maybe_inits: None };
 
     visitor.visit_body(body);
 }
 
+type MaybeInits<'mir, 'tcx> = ResultsCursor<
+    'mir,
+    'tcx,
+    dataflow::impls::MaybeInitializedPlaces<'mir, 'tcx, MoveDataParamEnv<'tcx>>,
+>;
+
 struct CheckLiveDrops<'mir, 'tcx> {
     ccx: &'mir ConstCx<'mir, 'tcx>,
     qualifs: Qualifs<'mir, 'tcx>,
+
+    maybe_inits: Option<MaybeInits<'mir, 'tcx>>,
 }
 
 // So we can access `body` and `tcx`.
@@ -83,7 +94,41 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
                     return;
                 }
 
-                if self.qualifs.needs_drop(self.ccx, dropped_place.local, location) {
+                if !self.qualifs.needs_drop(self.ccx, dropped_place.local, location) {
+                    return;
+                }
+
+                let ConstCx { param_env, body, tcx, def_id, .. } = *self.ccx;
+
+                // Replicate some logic from drop elaboration during const-checking. If we know
+                // that the active variant of an enum does not have drop glue, we can allow it to
+                // be dropped.
+                let maybe_inits = self.maybe_inits.get_or_insert_with(|| {
+                    let move_data = MoveData::gather_moves(body, tcx, param_env).unwrap();
+                    let mdpe = MoveDataParamEnv { move_data, param_env };
+                    dataflow::impls::MaybeInitializedPlaces::new(tcx, body, mdpe)
+                        .mark_inactive_variants_as_uninit(true)
+                        .into_engine(tcx, body, def_id.to_def_id())
+                        .iterate_to_fixpoint()
+                        .into_results_cursor(body)
+                });
+                maybe_inits.seek_before_primary_effect(location);
+                let mdpe = &maybe_inits.analysis().mdpe;
+
+                let dropped_mpi = mdpe
+                    .move_data
+                    .rev_lookup
+                    .find(dropped_place.as_ref())
+                    .expect_exact("All dropped places should have a move path");
+
+                let mut is_live_drop = false;
+                on_all_drop_children_bits(tcx, body, mdpe, dropped_mpi, |mpi| {
+                    if maybe_inits.contains(mpi) {
+                        is_live_drop = true;
+                    }
+                });
+
+                if is_live_drop {
                     // Use the span where the dropped local was declared for the error.
                     let span = self.body.local_decls[dropped_place.local].source_info.span;
                     self.check_live_drop(span);

src/librustc_mir/transform/elaborate_drops.rs

@@ -42,13 +42,14 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops {
             let dead_unwinds = find_dead_unwinds(tcx, body, def_id, &env);
 
             let inits = MaybeInitializedPlaces::new(tcx, body, &env)
+                .mark_inactive_variants_as_uninit(false)
                 .into_engine(tcx, body, def_id)
                 .dead_unwinds(&dead_unwinds)
                 .iterate_to_fixpoint()
                 .into_results_cursor(body);
 
             let uninits = MaybeUninitializedPlaces::new(tcx, body, &env)
-                .mark_inactive_variants_as_uninit()
+                .mark_inactive_variants_as_uninit(false)
                 .into_engine(tcx, body, def_id)
                 .dead_unwinds(&dead_unwinds)
                 .iterate_to_fixpoint()
@@ -81,7 +82,8 @@ fn find_dead_unwinds<'tcx>(
     // We only need to do this pass once, because unwind edges can only
     // reach cleanup blocks, which can't have unwind edges themselves.
     let mut dead_unwinds = BitSet::new_empty(body.basic_blocks().len());
-    let mut flow_inits = MaybeInitializedPlaces::new(tcx, body, &env)
+    let mut flow_inits = MaybeInitializedPlaces::new(tcx, body, env)
+        .mark_inactive_variants_as_uninit(false)
         .into_engine(tcx, body, def_id)
         .iterate_to_fixpoint()
         .into_results_cursor(body);

src/test/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-elaborate-drops.after.mir

@@ -8,19 +8,22 @@ fn unwrap(_1: std::option::Option<T>) -> T {
     let mut _4: !;                       // in scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL
     let mut _5: isize;                   // in scope 0 at $DIR/no-drop-for-inactive-variant.rs:12:1: 12:2
     let mut _6: isize;                   // in scope 0 at $DIR/no-drop-for-inactive-variant.rs:12:1: 12:2
-    let mut _7: isize;                   // in scope 0 at $DIR/no-drop-for-inactive-variant.rs:12:1: 12:2
     scope 1 {
         debug x => _3;                   // in scope 1 at $DIR/no-drop-for-inactive-variant.rs:9:14: 9:15
     }
 
     bb0: {
         _2 = discriminant(_1);           // scope 0 at $DIR/no-drop-for-inactive-variant.rs:9:9: 9:16
-        switchInt(move _2) -> [0_isize: bb1, 1_isize: bb3, otherwise: bb2]; // scope 0 at $DIR/no-drop-for-inactive-variant.rs:9:9: 9:16
+        switchInt(move _2) -> [0_isize: bb2, 1_isize: bb4, otherwise: bb3]; // scope 0 at $DIR/no-drop-for-inactive-variant.rs:9:9: 9:16
     }
 
-    bb1: {
+    bb1 (cleanup): {
+        resume;                          // scope 0 at $DIR/no-drop-for-inactive-variant.rs:7:1: 12:2
+    }
+
+    bb2: {
         StorageLive(_4);                 // scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL
-        std::rt::begin_panic::<&str>(const "explicit panic") -> bb4; // scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL
+        std::rt::begin_panic::<&str>(const "explicit panic") -> bb5; // scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL
                                          // mir::Constant
                                          // + span: $SRC_DIR/std/src/macros.rs:LL:COL
                                          // + literal: Const { ty: fn(&str) -> ! {std::rt::begin_panic::<&str>}, val: Value(Scalar(<ZST>)) }
@@ -32,21 +35,20 @@ fn unwrap(_1: std::option::Option<T>) -> T {
                                          // + literal: Const { ty: &str, val: Value(Slice { data: Allocation { bytes: [101, 120, 112, 108, 105, 99, 105, 116, 32, 112, 97, 110, 105, 99], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [16383], len: Size { raw: 14 } }, size: Size { raw: 14 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 14 }) }
     }
 
-    bb2: {
+    bb3: {
         unreachable;                     // scope 0 at $DIR/no-drop-for-inactive-variant.rs:8:11: 8:14
     }
 
-    bb3: {
+    bb4: {
         StorageLive(_3);                 // scope 0 at $DIR/no-drop-for-inactive-variant.rs:9:14: 9:15
         _3 = move ((_1 as Some).0: T);   // scope 0 at $DIR/no-drop-for-inactive-variant.rs:9:14: 9:15
         _0 = move _3;                    // scope 1 at $DIR/no-drop-for-inactive-variant.rs:9:20: 9:21
         StorageDead(_3);                 // scope 0 at $DIR/no-drop-for-inactive-variant.rs:9:20: 9:21
-        _6 = discriminant(_1);           // scope 0 at $DIR/no-drop-for-inactive-variant.rs:12:1: 12:2
+        _5 = discriminant(_1);           // scope 0 at $DIR/no-drop-for-inactive-variant.rs:12:1: 12:2
         return;                          // scope 0 at $DIR/no-drop-for-inactive-variant.rs:12:2: 12:2
     }
 
-    bb4 (cleanup): {
-        _5 = discriminant(_1);           // scope 0 at $DIR/no-drop-for-inactive-variant.rs:12:1: 12:2
-        resume;                          // scope 0 at $DIR/no-drop-for-inactive-variant.rs:7:1: 12:2
+    bb5 (cleanup): {
+        drop(_1) -> bb1;                 // scope 0 at $DIR/no-drop-for-inactive-variant.rs:12:1: 12:2
     }
 }