add test for GVN issue; cleanup in dataflow_const_prop

Author: RalfJung

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -260,8 +260,12 @@ impl<'tcx, Prov: Provenance> ImmTy<'tcx, Prov> {
         // This makes several assumptions about what layouts we will encounter; we match what
         // codegen does as good as we can (see `extract_field` in `rustc_codegen_ssa/src/mir/operand.rs`).
         let inner_val: Immediate<_> = match (**self, self.layout.abi) {
-            // if the entire value is uninit, then so is the field (can happen in ConstProp)
+            // If the entire value is uninit, then so is the field (can happen in ConstProp).
             (Immediate::Uninit, _) => Immediate::Uninit,
+            // If the field is uninhabited, we can forget the data (can happen in ConstProp).
+            // `enum S { A(!), B, C }` is an example of an enum with Scalar layout that
+            // has an `Uninhabited` variant, which means this case is possible.
+            _ if layout.abi.is_uninhabited() => Immediate::Uninit,
             // the field contains no information, can be left uninit
             // (Scalar/ScalarPair can contain even aligned ZST, not just 1-ZST)
             _ if layout.is_zst() => Immediate::Uninit,

compiler/rustc_mir_transform/src/dataflow_const_prop.rs

@@ -403,12 +403,7 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> {
             operand,
             &mut |elem, op| match elem {
                 TrackElem::Field(idx) => self.ecx.project_field(op, idx.as_usize()).ok(),
-                TrackElem::Variant(idx) => {
-                    if op.layout.for_variant(&self.ecx, idx).abi.is_uninhabited() {
-                        return None;
-                    }
-                    self.ecx.project_downcast(op, idx).ok()
-                }
+                TrackElem::Variant(idx) => self.ecx.project_downcast(op, idx).ok(),
                 TrackElem::Discriminant => {
                     let variant = self.ecx.read_discriminant(op).ok()?;
                     let discr_value =

tests/mir-opt/gvn_uninhabited.f.GVN.panic-abort.diff

@@ -0,0 +1,34 @@
+- // MIR for `f` before GVN
++ // MIR for `f` after GVN
+  
+  fn f() -> u32 {
+      let mut _0: u32;
+      let _1: u32;
+      let mut _2: E;
+      let mut _3: &U;
+      let _4: U;
+      scope 1 {
+          debug i => _1;
+      }
+      scope 2 {
+          let mut _5: &U;
+      }
+  
+      bb0: {
+          StorageLive(_2);
+          StorageLive(_3);
+          _5 = const _;
+          _3 = &(*_5);
+          _2 = ((*_3).1: E);
+          StorageLive(_1);
+-         _1 = ((_2 as A).1: u32);
++         _1 = const 0_u32;
+          StorageDead(_3);
+          StorageDead(_2);
+-         _0 = _1;
++         _0 = const 0_u32;
+          StorageDead(_1);
+          return;
+      }
+  }
+  

tests/mir-opt/gvn_uninhabited.f.GVN.panic-unwind.diff

@@ -0,0 +1,34 @@
+- // MIR for `f` before GVN
++ // MIR for `f` after GVN
+  
+  fn f() -> u32 {
+      let mut _0: u32;
+      let _1: u32;
+      let mut _2: E;
+      let mut _3: &U;
+      let _4: U;
+      scope 1 {
+          debug i => _1;
+      }
+      scope 2 {
+          let mut _5: &U;
+      }
+  
+      bb0: {
+          StorageLive(_2);
+          StorageLive(_3);
+          _5 = const _;
+          _3 = &(*_5);
+          _2 = ((*_3).1: E);
+          StorageLive(_1);
+-         _1 = ((_2 as A).1: u32);
++         _1 = const 0_u32;
+          StorageDead(_3);
+          StorageDead(_2);
+-         _0 = _1;
++         _0 = const 0_u32;
+          StorageDead(_1);
+          return;
+      }
+  }
+  

tests/mir-opt/gvn_uninhabited.rs

@@ -0,0 +1,24 @@
+// unit-test: GVN
+// compile-flags: -O
+// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
+// skip-filecheck
+
+#![feature(never_type)]
+
+#[derive(Copy, Clone)]
+pub enum E {
+    A(!, u32),
+}
+
+pub union U {
+    i: u32,
+    e: E,
+}
+
+// EMIT_MIR gvn_uninhabited.f.GVN.diff
+pub const fn f() -> u32 {
+    let E::A(_, i) = unsafe { (&U { i: 0 }).e };
+    i
+}
+
+fn main() {}