Add new MutatatingUseContexts for deinit and SetDiscriminant

Author: JakobDegen

compiler/rustc_borrowck/src/def_use.rs

@@ -72,5 +72,9 @@ pub fn categorize(context: PlaceContext) -> Option<DefUse> {
 
         // Debug info is neither def nor use.
         PlaceContext::NonUse(NonUseContext::VarDebugInfo) => None,
+
+        PlaceContext::MutatingUse(MutatingUseContext::Deinit | MutatingUseContext::SetDiscriminant) => {
+            bug!("These statements are not allowed in this MIR phase")
+        }
     }
 }

compiler/rustc_codegen_ssa/src/mir/analyze.rs

@@ -211,6 +211,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
 
             PlaceContext::MutatingUse(
                 MutatingUseContext::Store
+                | MutatingUseContext::Deinit
+                | MutatingUseContext::SetDiscriminant
                 | MutatingUseContext::AsmOutput
                 | MutatingUseContext::Borrow
                 | MutatingUseContext::AddressOf

compiler/rustc_middle/src/mir/visit.rs

@@ -395,14 +395,14 @@ macro_rules! make_mir_visitor {
                     StatementKind::SetDiscriminant { place, .. } => {
                         self.visit_place(
                             place,
-                            PlaceContext::MutatingUse(MutatingUseContext::Store),
+                            PlaceContext::MutatingUse(MutatingUseContext::SetDiscriminant),
                             location
                         );
                     }
                     StatementKind::Deinit(place) => {
                         self.visit_place(
                             place,
-                            PlaceContext::MutatingUse(MutatingUseContext::Store),
+                            PlaceContext::MutatingUse(MutatingUseContext::Deinit),
                             location
                         )
                     }
@@ -1181,6 +1181,10 @@ pub enum NonMutatingUseContext {
 pub enum MutatingUseContext {
     /// Appears as LHS of an assignment.
     Store,
+    /// Appears on `SetDiscriminant`
+    SetDiscriminant,
+    /// Appears on `Deinit`
+    Deinit,
     /// Output operand of an inline assembly block.
     AsmOutput,
     /// Destination of a call.

compiler/rustc_mir_dataflow/src/impls/init_locals.rs

@@ -77,6 +77,10 @@ impl<T> Visitor<'_> for TransferFunction<'_, T>
 where
     T: GenKill<Local>,
 {
+    // FIXME: Using `visit_local` here is a bug. For example, on `move _5.field` we mark `_5` as
+    // deinitialized, although clearly it is only partially deinitialized. This analysis is not
+    // actually used anywhere at the moment, so this is not critical, but this does need to be fixed
+    // before it starts being used again.
     fn visit_local(&mut self, &local: &Local, context: PlaceContext, _: Location) {
         use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, NonUseContext};
         match context {
@@ -87,6 +91,9 @@ where
                 | MutatingUseContext::Yield,
             ) => {}
 
+            // If it's deinitialized, it's no longer init
+            PlaceContext::MutatingUse(MutatingUseContext::Deinit) => self.trans.kill(local),
+
             // Otherwise, when a place is mutated, we must consider it possibly initialized.
             PlaceContext::MutatingUse(_) => self.trans.gen(local),
 

compiler/rustc_mir_dataflow/src/impls/liveness.rs

@@ -18,30 +18,6 @@ use crate::{AnalysisDomain, Backward, CallReturnPlaces, GenKill, GenKillAnalysis
 /// such an assignment is currently marked as a "use" of `x` in an attempt to be maximally
 /// conservative.
 ///
-/// ## Enums and `SetDiscriminant`
-///
-/// Assigning a literal value to an `enum` (e.g. `Option<i32>`), does not result in a simple
-/// assignment of the form `_1 = /*...*/` in the MIR. For example, the following assignment to `x`:
-///
-/// ```
-/// x = Some(4);
-/// ```
-///
-/// compiles to this MIR
-///
-/// ```
-/// ((_1 as Some).0: i32) = const 4_i32;
-/// discriminant(_1) = 1;
-/// ```
-///
-/// However, `MaybeLiveLocals` **does** mark `x` (`_1`) as "killed" after a statement like this.
-/// That's because it treats the `SetDiscriminant` operation as a definition of `x`, even though
-/// the writes that actually initialized the locals happened earlier.
-///
-/// This makes `MaybeLiveLocals` unsuitable for certain classes of optimization normally associated
-/// with a live variables analysis, notably dead-store elimination. It's a dirty hack, but it works
-/// okay for the generator state transform (currently the main consumer of this analysis).
-///
 /// [`MaybeBorrowedLocals`]: super::MaybeBorrowedLocals
 /// [flow-test]: https://github.com/rust-lang/rust/blob/a08c47310c7d49cbdc5d7afb38408ba519967ecd/src/test/ui/mir-dataflow/liveness-ptr.rs
 /// [liveness]: https://en.wikipedia.org/wiki/Live_variable_analysis
@@ -161,7 +137,13 @@ impl DefUse {
         match context {
             PlaceContext::NonUse(_) => None,
 
-            PlaceContext::MutatingUse(MutatingUseContext::Store) => Some(DefUse::Def),
+            PlaceContext::MutatingUse(MutatingUseContext::Store | MutatingUseContext::Deinit) => {
+                Some(DefUse::Def)
+            }
+
+            // Setting the discriminant is not a use because it does no reading, but it is also not
+            // a def because it does not overwrite the whole place
+            PlaceContext::MutatingUse(MutatingUseContext::SetDiscriminant) => None,
 
             // `MutatingUseContext::Call` and `MutatingUseContext::Yield` indicate that this is the
             // destination place for a `Call` return or `Yield` resume respectively. Since this is

compiler/rustc_mir_transform/src/const_prop.rs

@@ -897,8 +897,10 @@ impl Visitor<'_> for CanConstProp {
             // mutations of the same local via `Store`
             | MutatingUse(MutatingUseContext::Call)
             | MutatingUse(MutatingUseContext::AsmOutput)
+            | MutatingUse(MutatingUseContext::Deinit)
             // Actual store that can possibly even propagate a value
-            | MutatingUse(MutatingUseContext::Store) => {
+            | MutatingUse(MutatingUseContext::Store)
+            | MutatingUse(MutatingUseContext::SetDiscriminant) => {
                 if !self.found_assignment.insert(local) {
                     match &mut self.can_const_prop[local] {
                         // If the local can only get propagated in its own block, then we don't have

compiler/rustc_mir_transform/src/const_prop_lint.rs

@@ -778,7 +778,9 @@ impl Visitor<'_> for CanConstProp {
             // mutations of the same local via `Store`
             | MutatingUse(MutatingUseContext::Call)
             | MutatingUse(MutatingUseContext::AsmOutput)
+            | MutatingUse(MutatingUseContext::Deinit)
             // Actual store that can possibly even propagate a value
+            | MutatingUse(MutatingUseContext::SetDiscriminant)
             | MutatingUse(MutatingUseContext::Store) => {
                 if !self.found_assignment.insert(local) {
                     match &mut self.can_const_prop[local] {

src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff

@@ -17,24 +17,30 @@
       }
 
       bb0: {
-          StorageLive(_1);                 // scope 0 at $DIR/union.rs:13:9: 13:11
-          StorageLive(_2);                 // scope 0 at $DIR/union.rs:13:23: 13:28
-          _2 = val() -> bb1;               // scope 0 at $DIR/union.rs:13:23: 13:28
+-         StorageLive(_1);                 // scope 0 at $DIR/union.rs:13:9: 13:11
+-         StorageLive(_2);                 // scope 0 at $DIR/union.rs:13:23: 13:28
+-         _2 = val() -> bb1;               // scope 0 at $DIR/union.rs:13:23: 13:28
++         nop;                             // scope 0 at $DIR/union.rs:13:9: 13:11
++         nop;                             // scope 0 at $DIR/union.rs:13:23: 13:28
++         (_1.0: u32) = val() -> bb1;      // scope 0 at $DIR/union.rs:13:23: 13:28
                                            // mir::Constant
                                            // + span: $DIR/union.rs:13:23: 13:26
                                            // + literal: Const { ty: fn() -> u32 {val}, val: Value(Scalar(<ZST>)) }
       }
 
       bb1: {
           Deinit(_1);                      // scope 0 at $DIR/union.rs:13:14: 13:30
-          (_1.0: u32) = move _2;           // scope 0 at $DIR/union.rs:13:14: 13:30
-          StorageDead(_2);                 // scope 0 at $DIR/union.rs:13:29: 13:30
+-         (_1.0: u32) = move _2;           // scope 0 at $DIR/union.rs:13:14: 13:30
+-         StorageDead(_2);                 // scope 0 at $DIR/union.rs:13:29: 13:30
++         nop;                             // scope 0 at $DIR/union.rs:13:14: 13:30
++         nop;                             // scope 0 at $DIR/union.rs:13:29: 13:30
           StorageLive(_3);                 // scope 1 at $DIR/union.rs:15:5: 15:27
           StorageLive(_4);                 // scope 1 at $DIR/union.rs:15:10: 15:26
           _4 = (_1.0: u32);                // scope 2 at $DIR/union.rs:15:19: 15:24
           StorageDead(_4);                 // scope 1 at $DIR/union.rs:15:26: 15:27
           StorageDead(_3);                 // scope 1 at $DIR/union.rs:15:27: 15:28
-          StorageDead(_1);                 // scope 0 at $DIR/union.rs:16:1: 16:2
+-         StorageDead(_1);                 // scope 0 at $DIR/union.rs:16:1: 16:2
++         nop;                             // scope 0 at $DIR/union.rs:16:1: 16:2
           return;                          // scope 0 at $DIR/union.rs:16:2: 16:2
       }
   }

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

@@ -65,17 +65,22 @@
 
       bb0: {
 -         StorageLive(_3);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:8: 27:6
+-         StorageLive(_4);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
+-         StorageLive(_5);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
+-         _5 = _1;                         // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
 +         nop;                             // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:8: 27:6
-          StorageLive(_4);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
-          StorageLive(_5);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
-          _5 = _1;                         // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
++         nop;                             // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
++         nop;                             // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
++         (_4.0: &ViewportPercentageLength) = _1; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
           StorageLive(_6);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:18: 21:23
           _6 = _2;                         // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:18: 21:23
           Deinit(_4);                      // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
-          (_4.0: &ViewportPercentageLength) = move _5; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
+-         (_4.0: &ViewportPercentageLength) = move _5; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
++         nop;                             // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
           (_4.1: &ViewportPercentageLength) = move _6; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
           StorageDead(_6);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:23: 21:24
-          StorageDead(_5);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:23: 21:24
+-         StorageDead(_5);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:23: 21:24
++         nop;                             // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:23: 21:24
           _11 = discriminant((*(_4.0: &ViewportPercentageLength))); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
 -         switchInt(move _11) -> [0_isize: bb1, 1_isize: bb3, 2_isize: bb4, 3_isize: bb5, otherwise: bb11]; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:8: 21:24
 +         StorageLive(_34);                // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:8: 21:24
@@ -100,8 +105,9 @@
           discriminant(_0) = 1;            // scope 0 at $DIR/early_otherwise_branch_68867.rs:26:21: 26:28
           StorageDead(_33);                // scope 0 at $DIR/early_otherwise_branch_68867.rs:26:27: 26:28
 -         StorageDead(_3);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:27:6: 27:7
+-         StorageDead(_4);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
 +         nop;                             // scope 0 at $DIR/early_otherwise_branch_68867.rs:27:6: 27:7
-          StorageDead(_4);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
++         nop;                             // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
           return;                          // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:2: 28:2
       }
 
@@ -293,8 +299,9 @@
 +         nop;                             // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:5: 27:7
           discriminant(_0) = 0;            // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:5: 27:7
 -         StorageDead(_3);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:27:6: 27:7
+-         StorageDead(_4);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
 +         nop;                             // scope 0 at $DIR/early_otherwise_branch_68867.rs:27:6: 27:7
-          StorageDead(_4);                 // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
++         nop;                             // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
           return;                          // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:2: 28:2
       }