From 0f18203e85cfa1a140a53938487bbf82917c2e6b Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 14 Apr 2020 12:49:15 +0200
Subject: [PATCH 1/7] Miri: refactor read_discriminant and make it return
 Scalar

---
 src/librustc_middle/mir/interpret/error.rs |   4 +-
 src/librustc_mir/interpret/intrinsics.rs   |  10 +-
 src/librustc_mir/interpret/operand.rs      | 139 ++++++++++++---------
 src/librustc_mir/interpret/step.rs         |   3 +-
 4 files changed, 86 insertions(+), 70 deletions(-)

diff --git a/src/librustc_middle/mir/interpret/error.rs b/src/librustc_middle/mir/interpret/error.rs
index d32a147344992..fc588e049d7d8 100644
--- a/src/librustc_middle/mir/interpret/error.rs
+++ b/src/librustc_middle/mir/interpret/error.rs
@@ -1,4 +1,4 @@
-use super::{AllocId, Pointer, RawConst, ScalarMaybeUninit};
+use super::{AllocId, Pointer, RawConst, Scalar};
 
 use crate::mir::interpret::ConstValue;
 use crate::ty::layout::LayoutError;
@@ -391,7 +391,7 @@ pub enum UndefinedBehaviorInfo<'tcx> {
     /// Using a non-character `u32` as character.
     InvalidChar(u32),
     /// An enum discriminant was set to a value which was outside the range of valid values.
-    InvalidDiscriminant(ScalarMaybeUninit),
+    InvalidDiscriminant(Scalar),
     /// Using a pointer-not-to-a-function as function pointer.
     InvalidFunctionPointer(Pointer),
     /// Using a string that is not valid UTF-8,
diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs
index fc4be82ad90ad..6c7db9fce4cf0 100644
--- a/src/librustc_mir/interpret/intrinsics.rs
+++ b/src/librustc_mir/interpret/intrinsics.rs
@@ -218,15 +218,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             sym::discriminant_value => {
                 let place = self.deref_operand(args[0])?;
                 let discr_val = self.read_discriminant(place.into())?.0;
-                let scalar = match dest.layout.ty.kind {
-                    ty::Int(_) => Scalar::from_int(
-                        self.sign_extend(discr_val, dest.layout) as i128,
-                        dest.layout.size,
-                    ),
-                    ty::Uint(_) => Scalar::from_uint(discr_val, dest.layout.size),
-                    _ => bug!("invalid `discriminant_value` return layout: {:?}", dest.layout),
-                };
-                self.write_scalar(scalar, dest)?;
+                self.write_scalar(discr_val, dest)?;
             }
             sym::unchecked_shl
             | sym::unchecked_shr
diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index a3caa2048a1e7..7ad16a051fdec 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -15,8 +15,8 @@ use rustc_target::abi::{Abi, DiscriminantKind, HasDataLayout, Integer, LayoutOf,
 use rustc_target::abi::{VariantIdx, Variants};
 
 use super::{
-    from_known_layout, sign_extend, truncate, ConstValue, GlobalId, InterpCx, InterpResult,
-    MPlaceTy, Machine, MemPlace, Place, PlaceTy, Pointer, Scalar, ScalarMaybeUninit,
+    from_known_layout, ConstValue, GlobalId, InterpCx, InterpResult, MPlaceTy, Machine, MemPlace,
+    Place, PlaceTy, Pointer, Scalar, ScalarMaybeUninit,
 };
 
 /// An `Immediate` represents a single immediate self-contained Rust value.
@@ -577,91 +577,112 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     pub fn read_discriminant(
         &self,
         rval: OpTy<'tcx, M::PointerTag>,
-    ) -> InterpResult<'tcx, (u128, VariantIdx)> {
+    ) -> InterpResult<'tcx, (Scalar<M::PointerTag>, VariantIdx)> {
         trace!("read_discriminant_value {:#?}", rval.layout);
 
-        let (discr_layout, discr_kind, discr_index) = match rval.layout.variants {
+        let (discr_scalar_layout, discr_kind, discr_index) = match rval.layout.variants {
             Variants::Single { index } => {
-                let discr_val = rval
-                    .layout
-                    .ty
-                    .discriminant_for_variant(*self.tcx, index)
-                    .map_or(u128::from(index.as_u32()), |discr| discr.val);
-                return Ok((discr_val, index));
+                let discr = match rval.layout.ty.discriminant_for_variant(*self.tcx, index) {
+                    Some(discr) => {
+                        // This type actually has discriminants.
+                        let discr_layout = self.layout_of(discr.ty)?;
+                        Scalar::from_uint(discr.val, discr_layout.size)
+                    }
+                    None => {
+                        // On a type without actual discriminants, return variant idx as `u8`.
+                        let discr_layout = self.layout_of(self.tcx.types.u8)?;
+                        Scalar::from_uint(index.as_u32(), discr_layout.size)
+                    }
+                };
+                return Ok((discr, index));
             }
-            Variants::Multiple { discr: ref discr_layout, ref discr_kind, discr_index, .. } => {
-                (discr_layout, discr_kind, discr_index)
+            Variants::Multiple { ref discr, ref discr_kind, discr_index, .. } => {
+                (discr, discr_kind, discr_index)
             }
         };
 
-        // read raw discriminant value
-        let discr_op = self.operand_field(rval, discr_index)?;
-        let discr_val = self.read_immediate(discr_op)?;
-        let raw_discr = discr_val.to_scalar_or_undef();
-        trace!("discr value: {:?}", raw_discr);
-        // post-process
+        // There are *three* types/layouts that come into play here:
+        // - The field storing the discriminant has a layout, which my be a pointer.
+        //   This is `discr_val.layout`; we just use it for sanity checks.
+        // - The discriminant has a layout for tag storing purposes, which is always an integer.
+        //   This is `discr_layout` and is used to interpret the value we read from the
+        //   discriminant field.
+        // - The discriminant also has a type for typechecking, and that type's
+        //   layout can be *different*. This is `discr_ty`, and is used for the `Scalar`
+        //   we return. If necessary, a cast from `discr_layout` is performed.
+
+        // Get layout for tag.
+        let discr_layout = self.layout_of(discr_scalar_layout.value.to_int_ty(*self.tcx))?;
+
+        // Read discriminant value and sanity-check `discr_layout`.
+        let discr_val = self.read_immediate(self.operand_field(rval, discr_index)?)?;
+        assert_eq!(discr_layout.size, discr_val.layout.size);
+        assert_eq!(discr_layout.abi.is_signed(), discr_val.layout.abi.is_signed());
+        let discr_val = discr_val.to_scalar()?;
+        trace!("discriminant value: {:?}", discr_val);
+
+        // Get type used by typechecking.
+        let discr_ty = match rval.layout.ty.kind {
+            ty::Adt(adt, _) => {
+                let discr_int_ty = Integer::from_attr(self, adt.repr.discr_type());
+                // The signedness of tag and discriminant is the same.
+                discr_int_ty.to_ty(*self.tcx, discr_layout.abi.is_signed())
+            }
+            ty::Generator(_, substs, _) => {
+                let substs = substs.as_generator();
+                substs.discr_ty(*self.tcx)
+            }
+            _ => bug!("multiple variants for non-adt non-generator"),
+        };
+
+        // Figure out which discriminant and variant this corresponds to.
         Ok(match *discr_kind {
             DiscriminantKind::Tag => {
-                let bits_discr = raw_discr
-                    .not_undef()
-                    .and_then(|raw_discr| self.force_bits(raw_discr, discr_val.layout.size))
-                    .map_err(|_| err_ub!(InvalidDiscriminant(raw_discr.erase_tag())))?;
-                let real_discr = if discr_val.layout.abi.is_signed() {
-                    // going from layout tag type to typeck discriminant type
-                    // requires first sign extending with the discriminant layout
-                    let sexted = sign_extend(bits_discr, discr_val.layout.size);
-                    // and then zeroing with the typeck discriminant type
-                    let discr_ty = rval
-                        .layout
-                        .ty
-                        .ty_adt_def()
-                        .expect("tagged layout corresponds to adt")
-                        .repr
-                        .discr_type();
-                    let size = Integer::from_attr(self, discr_ty).size();
-                    truncate(sexted, size)
-                } else {
-                    bits_discr
-                };
-                // Make sure we catch invalid discriminants
+                let discr_bits = self
+                    .force_bits(discr_val, discr_layout.size)
+                    .map_err(|_| err_ub!(InvalidDiscriminant(discr_val.erase_tag())))?;
+                // Cast discriminant bits to the right type.
+                let discr_ty_layout = self.layout_of(discr_ty)?;
+                let discr_val_cast =
+                    self.cast_from_scalar(discr_bits, discr_layout, discr_ty);
+                let discr_bits = discr_val_cast.assert_bits(discr_ty_layout.size);
+                // Find variant index for this tag, and catch invalid discriminants.
                 let index = match rval.layout.ty.kind {
                     ty::Adt(adt, _) => {
-                        adt.discriminants(self.tcx.tcx).find(|(_, var)| var.val == real_discr)
+                        adt.discriminants(self.tcx.tcx).find(|(_, var)| var.val == discr_bits)
                     }
                     ty::Generator(def_id, substs, _) => {
                         let substs = substs.as_generator();
                         substs
                             .discriminants(def_id, self.tcx.tcx)
-                            .find(|(_, var)| var.val == real_discr)
+                            .find(|(_, var)| var.val == discr_bits)
                     }
                     _ => bug!("tagged layout for non-adt non-generator"),
                 }
-                .ok_or_else(|| err_ub!(InvalidDiscriminant(raw_discr.erase_tag())))?;
-                (real_discr, index.0)
+                .ok_or_else(|| err_ub!(InvalidDiscriminant(discr_val.erase_tag())))?;
+                // Return the cast value, and the index.
+                (discr_val_cast, index.0)
             }
             DiscriminantKind::Niche { dataful_variant, ref niche_variants, niche_start } => {
+                // Compute the variant this discriminant corresponds to. With niche layout,
+                // tag and variant index are the same.
                 let variants_start = niche_variants.start().as_u32();
                 let variants_end = niche_variants.end().as_u32();
-                let raw_discr = raw_discr
-                    .not_undef()
-                    .map_err(|_| err_ub!(InvalidDiscriminant(ScalarMaybeUninit::Uninit)))?;
-                match raw_discr.to_bits_or_ptr(discr_val.layout.size, self) {
+                let variant = match discr_val.to_bits_or_ptr(discr_layout.size, self) {
                     Err(ptr) => {
                         // The niche must be just 0 (which an inbounds pointer value never is)
                         let ptr_valid = niche_start == 0
                             && variants_start == variants_end
                             && !self.memory.ptr_may_be_null(ptr);
                         if !ptr_valid {
-                            throw_ub!(InvalidDiscriminant(raw_discr.erase_tag().into()))
+                            throw_ub!(InvalidDiscriminant(discr_val.erase_tag()))
                         }
-                        (u128::from(dataful_variant.as_u32()), dataful_variant)
+                        dataful_variant
                     }
-                    Ok(raw_discr) => {
+                    Ok(bits_discr) => {
                         // We need to use machine arithmetic to get the relative variant idx:
                         // variant_index_relative = discr_val - niche_start_val
-                        let discr_layout =
-                            self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?;
-                        let discr_val = ImmTy::from_uint(raw_discr, discr_layout);
+                        let discr_val = ImmTy::from_uint(bits_discr, discr_layout);
                         let niche_start_val = ImmTy::from_uint(niche_start, discr_layout);
                         let variant_index_relative_val =
                             self.binary_op(mir::BinOp::Sub, discr_val, niche_start_val)?;
@@ -684,12 +705,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                                 .variants
                                 .len();
                             assert!(usize::try_from(variant_index).unwrap() < variants_len);
-                            (u128::from(variant_index), VariantIdx::from_u32(variant_index))
+                            VariantIdx::from_u32(variant_index)
                         } else {
-                            (u128::from(dataful_variant.as_u32()), dataful_variant)
+                            dataful_variant
                         }
                     }
-                }
+                };
+                // Compute the size of the scalar we need to return.
+                // FIXME: Why do we not need to do a cast here like we do above?
+                let size = self.layout_of(discr_ty)?.size;
+                (Scalar::from_uint(variant.as_u32(), size), variant)
             }
         })
     }
diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs
index fd9815975c19f..bd4df788057e2 100644
--- a/src/librustc_mir/interpret/step.rs
+++ b/src/librustc_mir/interpret/step.rs
@@ -262,8 +262,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             Discriminant(place) => {
                 let op = self.eval_place_to_op(place, None)?;
                 let discr_val = self.read_discriminant(op)?.0;
-                let size = dest.layout.size;
-                self.write_scalar(Scalar::from_uint(discr_val, size), dest)?;
+                self.write_scalar(discr_val, dest)?;
             }
         }
 

From 5a3971cdb8694717f56b8c1bc2cd597bb4ac1677 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sat, 23 May 2020 12:10:13 +0200
Subject: [PATCH 2/7] comments and refactor variable names

---
 src/librustc_mir/interpret/operand.rs | 84 +++++++++++++++------------
 1 file changed, 46 insertions(+), 38 deletions(-)

diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index 7ad16a051fdec..17dfe8b656c1a 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -580,7 +580,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     ) -> InterpResult<'tcx, (Scalar<M::PointerTag>, VariantIdx)> {
         trace!("read_discriminant_value {:#?}", rval.layout);
 
-        let (discr_scalar_layout, discr_kind, discr_index) = match rval.layout.variants {
+        // We use "discriminant" to refer to the value associated with a particualr enum variant.
+        // This is not to be confused with its "variant index", which is just determining its position in the
+        // declared list of variants -- they can differ with explicitly assigned discriminants.
+        // We use "tag" to refer to how the discriminant is encoded in memory, which can be either
+        // straight-forward (`DiscriminantKind::Tag`) or with a niche (`DiscriminantKind::Niche`).
+        // Unfortunately, the rest of the compiler calls the latter "discriminant", too, which makes things
+        // rather confusing.
+        let (tag_scalar_layout, tag_kind, tag_index) = match rval.layout.variants {
             Variants::Single { index } => {
                 let discr = match rval.layout.ty.discriminant_for_variant(*self.tcx, index) {
                     Some(discr) => {
@@ -602,31 +609,31 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         };
 
         // There are *three* types/layouts that come into play here:
-        // - The field storing the discriminant has a layout, which my be a pointer.
-        //   This is `discr_val.layout`; we just use it for sanity checks.
-        // - The discriminant has a layout for tag storing purposes, which is always an integer.
-        //   This is `discr_layout` and is used to interpret the value we read from the
-        //   discriminant field.
-        // - The discriminant also has a type for typechecking, and that type's
-        //   layout can be *different*. This is `discr_ty`, and is used for the `Scalar`
-        //   we return. If necessary, a cast from `discr_layout` is performed.
+        // - The discriminant has a type for typechecking. This is `discr_ty`, and is used for
+        //   the `Scalar` we return.
+        // - The discriminant gets encoded as a tag/niche, with layout `tag_layout`.
+        //   This is always an integer, and used to interpret the value we read from the
+        //   tag field. For the return value, a cast to `discr_ty` is performed.
+        // - The field storing the tag has a layout, which is very similar to
+        //   `tag_layout` but may be a pointer. This is `tag_val.layout`;
+        //   we just use it for sanity checks.
 
         // Get layout for tag.
-        let discr_layout = self.layout_of(discr_scalar_layout.value.to_int_ty(*self.tcx))?;
+        let tag_layout = self.layout_of(tag_scalar_layout.value.to_int_ty(*self.tcx))?;
 
-        // Read discriminant value and sanity-check `discr_layout`.
-        let discr_val = self.read_immediate(self.operand_field(rval, discr_index)?)?;
-        assert_eq!(discr_layout.size, discr_val.layout.size);
-        assert_eq!(discr_layout.abi.is_signed(), discr_val.layout.abi.is_signed());
-        let discr_val = discr_val.to_scalar()?;
-        trace!("discriminant value: {:?}", discr_val);
+        // Read tag and sanity-check `tag_layout`.
+        let tag_val = self.read_immediate(self.operand_field(rval, tag_index)?)?;
+        assert_eq!(tag_layout.size, tag_val.layout.size);
+        assert_eq!(tag_layout.abi.is_signed(), tag_val.layout.abi.is_signed());
+        let tag_val = tag_val.to_scalar()?;
+        trace!("tag value: {:?}", tag_val);
 
         // Get type used by typechecking.
         let discr_ty = match rval.layout.ty.kind {
             ty::Adt(adt, _) => {
                 let discr_int_ty = Integer::from_attr(self, adt.repr.discr_type());
                 // The signedness of tag and discriminant is the same.
-                discr_int_ty.to_ty(*self.tcx, discr_layout.abi.is_signed())
+                discr_int_ty.to_ty(*self.tcx, tag_layout.abi.is_signed())
             }
             ty::Generator(_, substs, _) => {
                 let substs = substs.as_generator();
@@ -636,17 +643,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         };
 
         // Figure out which discriminant and variant this corresponds to.
-        Ok(match *discr_kind {
+        Ok(match *tag_kind {
             DiscriminantKind::Tag => {
-                let discr_bits = self
-                    .force_bits(discr_val, discr_layout.size)
-                    .map_err(|_| err_ub!(InvalidDiscriminant(discr_val.erase_tag())))?;
-                // Cast discriminant bits to the right type.
-                let discr_ty_layout = self.layout_of(discr_ty)?;
+                let tag_bits = self
+                    .force_bits(tag_val, tag_layout.size)
+                    .map_err(|_| err_ub!(InvalidDiscriminant(tag_val.erase_tag())))?;
+                // Cast bits from tag layout to discriminant layout.
+                let discr_layout = self.layout_of(discr_ty)?;
                 let discr_val_cast =
-                    self.cast_from_scalar(discr_bits, discr_layout, discr_ty);
-                let discr_bits = discr_val_cast.assert_bits(discr_ty_layout.size);
-                // Find variant index for this tag, and catch invalid discriminants.
+                    self.cast_from_scalar(tag_bits, tag_layout, discr_ty);
+                let discr_bits = discr_val_cast.assert_bits(discr_layout.size);
+                // Convert discriminant to variant index, and catch invalid discriminants.
                 let index = match rval.layout.ty.kind {
                     ty::Adt(adt, _) => {
                         adt.discriminants(self.tcx.tcx).find(|(_, var)| var.val == discr_bits)
@@ -659,36 +666,36 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     }
                     _ => bug!("tagged layout for non-adt non-generator"),
                 }
-                .ok_or_else(|| err_ub!(InvalidDiscriminant(discr_val.erase_tag())))?;
+                .ok_or_else(|| err_ub!(InvalidDiscriminant(tag_val.erase_tag())))?;
                 // Return the cast value, and the index.
                 (discr_val_cast, index.0)
             }
             DiscriminantKind::Niche { dataful_variant, ref niche_variants, niche_start } => {
-                // Compute the variant this discriminant corresponds to. With niche layout,
-                // tag and variant index are the same.
+                // Compute the variant this niche value/"tag" corresponds to. With niche layout,
+                // discriminant (encoded in niche/tag) and variant index are the same.
                 let variants_start = niche_variants.start().as_u32();
                 let variants_end = niche_variants.end().as_u32();
-                let variant = match discr_val.to_bits_or_ptr(discr_layout.size, self) {
+                let variant = match tag_val.to_bits_or_ptr(tag_layout.size, self) {
                     Err(ptr) => {
                         // The niche must be just 0 (which an inbounds pointer value never is)
                         let ptr_valid = niche_start == 0
                             && variants_start == variants_end
                             && !self.memory.ptr_may_be_null(ptr);
                         if !ptr_valid {
-                            throw_ub!(InvalidDiscriminant(discr_val.erase_tag()))
+                            throw_ub!(InvalidDiscriminant(tag_val.erase_tag()))
                         }
                         dataful_variant
                     }
-                    Ok(bits_discr) => {
+                    Ok(tag_bits) => {
                         // We need to use machine arithmetic to get the relative variant idx:
-                        // variant_index_relative = discr_val - niche_start_val
-                        let discr_val = ImmTy::from_uint(bits_discr, discr_layout);
-                        let niche_start_val = ImmTy::from_uint(niche_start, discr_layout);
+                        // variant_index_relative = tag_val - niche_start_val
+                        let tag_val = ImmTy::from_uint(tag_bits, tag_layout);
+                        let niche_start_val = ImmTy::from_uint(niche_start, tag_layout);
                         let variant_index_relative_val =
-                            self.binary_op(mir::BinOp::Sub, discr_val, niche_start_val)?;
+                            self.binary_op(mir::BinOp::Sub, tag_val, niche_start_val)?;
                         let variant_index_relative = variant_index_relative_val
                             .to_scalar()?
-                            .assert_bits(discr_val.layout.size);
+                            .assert_bits(tag_val.layout.size);
                         // Check if this is in the range that indicates an actual discriminant.
                         if variant_index_relative <= u128::from(variants_end - variants_start) {
                             let variant_index_relative = u32::try_from(variant_index_relative)
@@ -712,7 +719,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     }
                 };
                 // Compute the size of the scalar we need to return.
-                // FIXME: Why do we not need to do a cast here like we do above?
+                // No need to cast, because the variant index directly serves as discriminant and is
+                // encoded in the tag.
                 let size = self.layout_of(discr_ty)?.size;
                 (Scalar::from_uint(variant.as_u32(), size), variant)
             }

From f8f80334872cf63412b0fe2e79b87505af2ba6a1 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sat, 23 May 2020 13:22:13 +0200
Subject: [PATCH 3/7] assert that types without discriminant use variant idx of
 0

---
 src/librustc_mir/interpret/operand.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index 17dfe8b656c1a..81ef8172da77f 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -596,7 +596,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         Scalar::from_uint(discr.val, discr_layout.size)
                     }
                     None => {
-                        // On a type without actual discriminants, return variant idx as `u8`.
+                        // On a type without actual discriminants, variant is 0. Return variant idx as `u8`.
+                        assert_eq!(index.as_u32(), 0);
                         let discr_layout = self.layout_of(self.tcx.types.u8)?;
                         Scalar::from_uint(index.as_u32(), discr_layout.size)
                     }

From d94923ea469b4c104719071a82a4bc051fed77ac Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sat, 23 May 2020 14:51:50 +0200
Subject: [PATCH 4/7] Format and more tracing

---
 src/librustc_mir/interpret/operand.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index 81ef8172da77f..d46e6e387f508 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -642,6 +642,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
             _ => bug!("multiple variants for non-adt non-generator"),
         };
+        trace!("discriminant type: {:?}", discr_ty);
 
         // Figure out which discriminant and variant this corresponds to.
         Ok(match *tag_kind {
@@ -651,8 +652,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     .map_err(|_| err_ub!(InvalidDiscriminant(tag_val.erase_tag())))?;
                 // Cast bits from tag layout to discriminant layout.
                 let discr_layout = self.layout_of(discr_ty)?;
-                let discr_val_cast =
-                    self.cast_from_scalar(tag_bits, tag_layout, discr_ty);
+                let discr_val_cast = self.cast_from_scalar(tag_bits, tag_layout, discr_ty);
                 let discr_bits = discr_val_cast.assert_bits(discr_layout.size);
                 // Convert discriminant to variant index, and catch invalid discriminants.
                 let index = match rval.layout.ty.kind {

From 64fbe2fc485477406724a68372f4351dc7a08b0a Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sat, 23 May 2020 15:42:35 +0200
Subject: [PATCH 5/7] Add helper method for determining the type of a
 discriminant

---
 src/librustc_middle/mir/tcx.rs        | 11 +----
 src/librustc_middle/ty/sty.rs         | 10 +++++
 src/librustc_mir/interpret/operand.rs | 63 ++++++++++-----------------
 3 files changed, 35 insertions(+), 49 deletions(-)

diff --git a/src/librustc_middle/mir/tcx.rs b/src/librustc_middle/mir/tcx.rs
index 17edd9f4cb643..174da7db1753d 100644
--- a/src/librustc_middle/mir/tcx.rs
+++ b/src/librustc_middle/mir/tcx.rs
@@ -5,7 +5,6 @@
 
 use crate::mir::*;
 use crate::ty::subst::Subst;
-use crate::ty::util::IntTypeExt;
 use crate::ty::{self, Ty, TyCtxt};
 use rustc_hir as hir;
 use rustc_target::abi::VariantIdx;
@@ -175,15 +174,7 @@ impl<'tcx> Rvalue<'tcx> {
             }
             Rvalue::UnaryOp(UnOp::Not | UnOp::Neg, ref operand) => operand.ty(local_decls, tcx),
             Rvalue::Discriminant(ref place) => {
-                let ty = place.ty(local_decls, tcx).ty;
-                match ty.kind {
-                    ty::Adt(adt_def, _) => adt_def.repr.discr_type().to_ty(tcx),
-                    ty::Generator(_, substs, _) => substs.as_generator().discr_ty(tcx),
-                    _ => {
-                        // This can only be `0`, for now, so `u8` will suffice.
-                        tcx.types.u8
-                    }
-                }
+                place.ty(local_decls, tcx).ty.discriminant_type(tcx)
             }
             Rvalue::NullaryOp(NullOp::Box, t) => tcx.mk_box(t),
             Rvalue::NullaryOp(NullOp::SizeOf, _) => tcx.types.usize,
diff --git a/src/librustc_middle/ty/sty.rs b/src/librustc_middle/ty/sty.rs
index 370702f7f221d..6cee224219b63 100644
--- a/src/librustc_middle/ty/sty.rs
+++ b/src/librustc_middle/ty/sty.rs
@@ -29,6 +29,7 @@ use std::borrow::Cow;
 use std::cmp::Ordering;
 use std::marker::PhantomData;
 use std::ops::Range;
+use ty::util::IntTypeExt;
 
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, RustcEncodable, RustcDecodable)]
 #[derive(HashStable, TypeFoldable, Lift)]
@@ -2104,6 +2105,15 @@ impl<'tcx> TyS<'tcx> {
         }
     }
 
+    /// Returns the type of the discriminant of this type.
+    pub fn discriminant_type(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
+        match self.kind {
+            ty::Adt(adt_def, _) => adt_def.repr.discr_type().to_ty(tcx),
+            ty::Generator(_, substs, _) => substs.as_generator().discr_ty(tcx),
+            _ => bug!("{:?} does not have a discriminant", self),
+        }
+    }
+
     /// When we create a closure, we record its kind (i.e., what trait
     /// it implements) into its `ClosureSubsts` using a type
     /// parameter. This is kind of a phantom type, except that the
diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index d46e6e387f508..139871310fbf3 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -7,11 +7,11 @@ use std::fmt::Write;
 use rustc_errors::ErrorReported;
 use rustc_hir::def::Namespace;
 use rustc_macros::HashStable;
-use rustc_middle::ty::layout::{IntegerExt, PrimitiveExt, TyAndLayout};
+use rustc_middle::ty::layout::{PrimitiveExt, TyAndLayout};
 use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter, Printer};
 use rustc_middle::ty::Ty;
 use rustc_middle::{mir, ty};
-use rustc_target::abi::{Abi, DiscriminantKind, HasDataLayout, Integer, LayoutOf, Size};
+use rustc_target::abi::{Abi, DiscriminantKind, HasDataLayout, LayoutOf, Size};
 use rustc_target::abi::{VariantIdx, Variants};
 
 use super::{
@@ -576,9 +576,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// Read discriminant, return the runtime value as well as the variant index.
     pub fn read_discriminant(
         &self,
-        rval: OpTy<'tcx, M::PointerTag>,
+        op: OpTy<'tcx, M::PointerTag>,
     ) -> InterpResult<'tcx, (Scalar<M::PointerTag>, VariantIdx)> {
-        trace!("read_discriminant_value {:#?}", rval.layout);
+        trace!("read_discriminant_value {:#?}", op.layout);
+
+        // Get type and layout of the discriminant.
+        let discr_layout = self.layout_of(op.layout.ty.discriminant_type(*self.tcx))?;
+        trace!("discriminant type: {:?}", discr_layout.ty);
 
         // We use "discriminant" to refer to the value associated with a particualr enum variant.
         // This is not to be confused with its "variant index", which is just determining its position in the
@@ -587,18 +591,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // straight-forward (`DiscriminantKind::Tag`) or with a niche (`DiscriminantKind::Niche`).
         // Unfortunately, the rest of the compiler calls the latter "discriminant", too, which makes things
         // rather confusing.
-        let (tag_scalar_layout, tag_kind, tag_index) = match rval.layout.variants {
+        let (tag_scalar_layout, tag_kind, tag_index) = match op.layout.variants {
             Variants::Single { index } => {
-                let discr = match rval.layout.ty.discriminant_for_variant(*self.tcx, index) {
+                let discr = match op.layout.ty.discriminant_for_variant(*self.tcx, index) {
                     Some(discr) => {
                         // This type actually has discriminants.
-                        let discr_layout = self.layout_of(discr.ty)?;
+                        assert_eq!(discr.ty, discr_layout.ty);
                         Scalar::from_uint(discr.val, discr_layout.size)
                     }
                     None => {
-                        // On a type without actual discriminants, variant is 0. Return variant idx as `u8`.
+                        // On a type without actual discriminants, variant is 0.
                         assert_eq!(index.as_u32(), 0);
-                        let discr_layout = self.layout_of(self.tcx.types.u8)?;
                         Scalar::from_uint(index.as_u32(), discr_layout.size)
                     }
                 };
@@ -609,41 +612,25 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
         };
 
-        // There are *three* types/layouts that come into play here:
-        // - The discriminant has a type for typechecking. This is `discr_ty`, and is used for
+        // There are *three* layouts that come into play here:
+        // - The discriminant has a type for typechecking. This is `discr_layout`, and is used for
         //   the `Scalar` we return.
-        // - The discriminant gets encoded as a tag/niche, with layout `tag_layout`.
-        //   This is always an integer, and used to interpret the value we read from the
-        //   tag field. For the return value, a cast to `discr_ty` is performed.
-        // - The field storing the tag has a layout, which is very similar to
-        //   `tag_layout` but may be a pointer. This is `tag_val.layout`;
-        //   we just use it for sanity checks.
+        // - The tag (encoded discriminant) has layout `tag_layout`. This is always an integer type,
+        //   and used to interpret the value we read from the tag field.
+        //   For the return value, a cast to `discr_layout` is performed.
+        // - The field storing the tag has a layout, which is very similar to `tag_layout` but
+        //   may be a pointer. This is `tag_val.layout`; we just use it for sanity checks.
 
         // Get layout for tag.
         let tag_layout = self.layout_of(tag_scalar_layout.value.to_int_ty(*self.tcx))?;
 
         // Read tag and sanity-check `tag_layout`.
-        let tag_val = self.read_immediate(self.operand_field(rval, tag_index)?)?;
+        let tag_val = self.read_immediate(self.operand_field(op, tag_index)?)?;
         assert_eq!(tag_layout.size, tag_val.layout.size);
         assert_eq!(tag_layout.abi.is_signed(), tag_val.layout.abi.is_signed());
         let tag_val = tag_val.to_scalar()?;
         trace!("tag value: {:?}", tag_val);
 
-        // Get type used by typechecking.
-        let discr_ty = match rval.layout.ty.kind {
-            ty::Adt(adt, _) => {
-                let discr_int_ty = Integer::from_attr(self, adt.repr.discr_type());
-                // The signedness of tag and discriminant is the same.
-                discr_int_ty.to_ty(*self.tcx, tag_layout.abi.is_signed())
-            }
-            ty::Generator(_, substs, _) => {
-                let substs = substs.as_generator();
-                substs.discr_ty(*self.tcx)
-            }
-            _ => bug!("multiple variants for non-adt non-generator"),
-        };
-        trace!("discriminant type: {:?}", discr_ty);
-
         // Figure out which discriminant and variant this corresponds to.
         Ok(match *tag_kind {
             DiscriminantKind::Tag => {
@@ -651,11 +638,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     .force_bits(tag_val, tag_layout.size)
                     .map_err(|_| err_ub!(InvalidDiscriminant(tag_val.erase_tag())))?;
                 // Cast bits from tag layout to discriminant layout.
-                let discr_layout = self.layout_of(discr_ty)?;
-                let discr_val_cast = self.cast_from_scalar(tag_bits, tag_layout, discr_ty);
+                let discr_val_cast = self.cast_from_scalar(tag_bits, tag_layout, discr_layout.ty);
                 let discr_bits = discr_val_cast.assert_bits(discr_layout.size);
                 // Convert discriminant to variant index, and catch invalid discriminants.
-                let index = match rval.layout.ty.kind {
+                let index = match op.layout.ty.kind {
                     ty::Adt(adt, _) => {
                         adt.discriminants(self.tcx.tcx).find(|(_, var)| var.val == discr_bits)
                     }
@@ -705,7 +691,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                             let variant_index = variants_start
                                 .checked_add(variant_index_relative)
                                 .expect("overflow computing absolute variant idx");
-                            let variants_len = rval
+                            let variants_len = op
                                 .layout
                                 .ty
                                 .ty_adt_def()
@@ -722,8 +708,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 // Compute the size of the scalar we need to return.
                 // No need to cast, because the variant index directly serves as discriminant and is
                 // encoded in the tag.
-                let size = self.layout_of(discr_ty)?.size;
-                (Scalar::from_uint(variant.as_u32(), size), variant)
+                (Scalar::from_uint(variant.as_u32(), discr_layout.size), variant)
             }
         })
     }

From ad7179d2a409faaf45465862f44efe7f989cd71e Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sat, 23 May 2020 17:24:33 +0200
Subject: [PATCH 6/7] fix discriminant_ty for non-enums

---
 src/librustc_middle/mir/tcx.rs        |  4 +---
 src/librustc_middle/ty/mod.rs         |  5 +++++
 src/librustc_middle/ty/sty.rs         | 13 +++++++++----
 src/librustc_mir/interpret/operand.rs |  4 ++--
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/librustc_middle/mir/tcx.rs b/src/librustc_middle/mir/tcx.rs
index 174da7db1753d..3e172bd8d2a76 100644
--- a/src/librustc_middle/mir/tcx.rs
+++ b/src/librustc_middle/mir/tcx.rs
@@ -173,9 +173,7 @@ impl<'tcx> Rvalue<'tcx> {
                 tcx.intern_tup(&[ty, tcx.types.bool])
             }
             Rvalue::UnaryOp(UnOp::Not | UnOp::Neg, ref operand) => operand.ty(local_decls, tcx),
-            Rvalue::Discriminant(ref place) => {
-                place.ty(local_decls, tcx).ty.discriminant_type(tcx)
-            }
+            Rvalue::Discriminant(ref place) => place.ty(local_decls, tcx).ty.discriminant_ty(tcx),
             Rvalue::NullaryOp(NullOp::Box, t) => tcx.mk_box(t),
             Rvalue::NullaryOp(NullOp::SizeOf, _) => tcx.types.usize,
             Rvalue::Aggregate(ref ak, ref ops) => match **ak {
diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs
index aad3c6889c3ce..4d050a7b48eac 100644
--- a/src/librustc_middle/ty/mod.rs
+++ b/src/librustc_middle/ty/mod.rs
@@ -2040,6 +2040,8 @@ impl ReprOptions {
         self.flags.contains(ReprFlags::HIDE_NICHE)
     }
 
+    /// Returns the discriminant type, given these `repr` options.
+    /// This must only be called on enums!
     pub fn discr_type(&self) -> attr::IntType {
         self.int.unwrap_or(attr::SignedInt(ast::IntTy::Isize))
     }
@@ -2272,6 +2274,7 @@ impl<'tcx> AdtDef {
 
     #[inline]
     pub fn eval_explicit_discr(&self, tcx: TyCtxt<'tcx>, expr_did: DefId) -> Option<Discr<'tcx>> {
+        assert!(self.is_enum());
         let param_env = tcx.param_env(expr_did);
         let repr_type = self.repr.discr_type();
         match tcx.const_eval_poly(expr_did) {
@@ -2308,6 +2311,7 @@ impl<'tcx> AdtDef {
         &'tcx self,
         tcx: TyCtxt<'tcx>,
     ) -> impl Iterator<Item = (VariantIdx, Discr<'tcx>)> + Captures<'tcx> {
+        assert!(self.is_enum());
         let repr_type = self.repr.discr_type();
         let initial = repr_type.initial_discriminant(tcx);
         let mut prev_discr = None::<Discr<'tcx>>;
@@ -2340,6 +2344,7 @@ impl<'tcx> AdtDef {
         tcx: TyCtxt<'tcx>,
         variant_index: VariantIdx,
     ) -> Discr<'tcx> {
+        assert!(self.is_enum());
         let (val, offset) = self.discriminant_def_for_variant(variant_index);
         let explicit_value = val
             .and_then(|expr_did| self.eval_explicit_discr(tcx, expr_did))
diff --git a/src/librustc_middle/ty/sty.rs b/src/librustc_middle/ty/sty.rs
index 6cee224219b63..aacca710dd037 100644
--- a/src/librustc_middle/ty/sty.rs
+++ b/src/librustc_middle/ty/sty.rs
@@ -2097,7 +2097,9 @@ impl<'tcx> TyS<'tcx> {
         variant_index: VariantIdx,
     ) -> Option<Discr<'tcx>> {
         match self.kind {
-            TyKind::Adt(adt, _) => Some(adt.discriminant_for_variant(tcx, variant_index)),
+            TyKind::Adt(adt, _) if adt.is_enum() => {
+                Some(adt.discriminant_for_variant(tcx, variant_index))
+            }
             TyKind::Generator(def_id, substs, _) => {
                 Some(substs.as_generator().discriminant_for_variant(def_id, tcx, variant_index))
             }
@@ -2106,11 +2108,14 @@ impl<'tcx> TyS<'tcx> {
     }
 
     /// Returns the type of the discriminant of this type.
-    pub fn discriminant_type(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
+    pub fn discriminant_ty(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
         match self.kind {
-            ty::Adt(adt_def, _) => adt_def.repr.discr_type().to_ty(tcx),
+            ty::Adt(adt, _) if adt.is_enum() => adt.repr.discr_type().to_ty(tcx),
             ty::Generator(_, substs, _) => substs.as_generator().discr_ty(tcx),
-            _ => bug!("{:?} does not have a discriminant", self),
+            _ => {
+                // This can only be `0`, for now, so `u8` will suffice.
+                tcx.types.u8
+            }
         }
     }
 
diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index 139871310fbf3..f546f6236d777 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -581,10 +581,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         trace!("read_discriminant_value {:#?}", op.layout);
 
         // Get type and layout of the discriminant.
-        let discr_layout = self.layout_of(op.layout.ty.discriminant_type(*self.tcx))?;
+        let discr_layout = self.layout_of(op.layout.ty.discriminant_ty(*self.tcx))?;
         trace!("discriminant type: {:?}", discr_layout.ty);
 
-        // We use "discriminant" to refer to the value associated with a particualr enum variant.
+        // We use "discriminant" to refer to the value associated with a particular enum variant.
         // This is not to be confused with its "variant index", which is just determining its position in the
         // declared list of variants -- they can differ with explicitly assigned discriminants.
         // We use "tag" to refer to how the discriminant is encoded in memory, which can be either

From c4b6224ea46f57bb59df8d321d8f40e7f2900423 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sat, 30 May 2020 00:02:30 +0200
Subject: [PATCH 7/7] more type sanity checks in Miri

---
 src/librustc_mir/interpret/operand.rs | 12 ++++++++++--
 src/librustc_mir/interpret/place.rs   |  8 ++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index f546f6236d777..3cfc331ef8c4e 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -15,8 +15,8 @@ use rustc_target::abi::{Abi, DiscriminantKind, HasDataLayout, LayoutOf, Size};
 use rustc_target::abi::{VariantIdx, Variants};
 
 use super::{
-    from_known_layout, ConstValue, GlobalId, InterpCx, InterpResult, MPlaceTy, Machine, MemPlace,
-    Place, PlaceTy, Pointer, Scalar, ScalarMaybeUninit,
+    from_known_layout, mir_assign_valid_types, ConstValue, GlobalId, InterpCx, InterpResult,
+    MPlaceTy, Machine, MemPlace, Place, PlaceTy, Pointer, Scalar, ScalarMaybeUninit,
 };
 
 /// An `Immediate` represents a single immediate self-contained Rust value.
@@ -469,6 +469,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             .try_fold(base_op, |op, elem| self.operand_projection(op, elem))?;
 
         trace!("eval_place_to_op: got {:?}", *op);
+        // Sanity-check the type we ended up with.
+        debug_assert!(mir_assign_valid_types(
+            *self.tcx,
+            self.layout_of(self.subst_from_current_frame_and_normalize_erasing_regions(
+                place.ty(&self.frame().body.local_decls, *self.tcx).ty
+            ))?,
+            op.layout,
+        ));
         Ok(op)
     }
 
diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs
index 6dadb8e4c67f4..f5e7c1a4823cf 100644
--- a/src/librustc_mir/interpret/place.rs
+++ b/src/librustc_mir/interpret/place.rs
@@ -638,6 +638,14 @@ where
         }
 
         self.dump_place(place_ty.place);
+        // Sanity-check the type we ended up with.
+        debug_assert!(mir_assign_valid_types(
+            *self.tcx,
+            self.layout_of(self.subst_from_current_frame_and_normalize_erasing_regions(
+                place.ty(&self.frame().body.local_decls, *self.tcx).ty
+            ))?,
+            place_ty.layout,
+        ));
         Ok(place_ty)
     }