rustc: generalize layout::Variants::NicheFilling to niches other than 0.

src/librustc/ty/layout.rs

@@ -790,17 +790,18 @@ pub enum Variants {
         variants: Vec<CachedLayout>,
     },
 
-    /// Two cases distinguished by a niche: the case with discriminant
+    /// Two cases distinguished by a niche (a value invalid for a type):
-    /// `nndiscr` is represented by the struct `nonnull`, where field `0`
+    /// the variant `dataful_variant` contains a niche at an arbitrary
-    /// is known to be nonnull due to its type; if that field is null, then
+    /// offset (field 0 of the enum), which is set to `niche_value`
-    /// it represents the other case, which is known to be zero sized.
+    /// for the other variant.
     ///
-    /// For example, `std::option::Option` instantiated at a safe pointer type
+    /// For example, `Option<(usize, &T)>`  is represented such that
-    /// is represented such that `None` is a null pointer and `Some` is the
+    /// `None` has a null pointer for the second tuple field, and
-    /// identity function.
+    /// `Some` is the identity function (with a non-null reference).
     NicheFilling {
-        nndiscr: u64,
+        dataful_variant: usize,
-        discr: Primitive,
+        niche: Primitive,
+        niche_value: u128,
         variants: Vec<CachedLayout>,
     }
 }
@@ -1323,7 +1324,7 @@ impl<'a, 'tcx> CachedLayout {
                         }
 
                         for (field_index, field) in variants[i].iter().enumerate() {
-                            if let Some((offset, discr)) = field.non_zero_field(cx)? {
+                            if let Some((offset, niche, niche_value)) = field.find_niche(cx)? {
                                 let mut st = vec![
                                     univariant_uninterned(&variants[0],
                                         &def.repr, StructKind::AlwaysSized)?,
@@ -1342,23 +1343,23 @@ impl<'a, 'tcx> CachedLayout {
                                     ..
                                 } = st[i];
 
-                                let mut discr_align = discr.align(dl);
+                                let mut niche_align = niche.align(dl);
-                                if offset.bytes() == 0 && discr.size(dl) == size {
+                                if offset.bytes() == 0 && niche.size(dl) == size {
-                                    abi = Abi::Scalar(discr);
+                                    abi = Abi::Scalar(niche);
                                 } else if let Abi::Aggregate { ref mut packed, .. } = abi {
-                                    if offset.abi_align(discr_align) != offset {
+                                    if offset.abi_align(niche_align) != offset {
                                         *packed = true;
-                                        discr_align = dl.i8_align;
+                                        niche_align = dl.i8_align;
                                     }
                                 }
-                                align = align.max(discr_align);
+                                align = align.max(niche_align);
-                                primitive_align = primitive_align.max(discr_align);
+                                primitive_align = primitive_align.max(niche_align);
 
                                 return Ok(tcx.intern_layout(CachedLayout {
                                     variants: Variants::NicheFilling {
-                                        nndiscr: i as u64,
+                                        dataful_variant: i,
-
+                                        niche,
-                                        discr,
+                                        niche_value,
                                         variants: st,
                                     },
                                     fields: FieldPlacement::Arbitrary {
@@ -2048,7 +2049,7 @@ impl<'a, 'tcx> TyLayout<'tcx> {
 
                     // Discriminant field for enums (where applicable).
                     Variants::Tagged { discr, .. } |
-                    Variants::NicheFilling { discr, .. } => {
+                    Variants::NicheFilling { niche: discr, .. } => {
                         return cx.layout_of([discr.to_ty(tcx)][i]);
                     }
                 }
@@ -2084,30 +2085,48 @@ impl<'a, 'tcx> TyLayout<'tcx> {
         (self.size, self.align)
     }
 
-    /// Find the offset of a non-zero leaf field, starting from
+    /// Find the offset of a niche leaf field, starting from
     /// the given type and recursing through aggregates.
-    /// The tuple is `(offset, primitive, source_path)`.
+    /// The tuple is `(offset, primitive, niche_value)`.
     // FIXME(eddyb) track value ranges and traverse already optimized enums.
-    fn non_zero_field<C>(&self, cx: C)
+    fn find_niche<C>(&self, cx: C)
-        -> Result<Option<(Size, Primitive)>, LayoutError<'tcx>>
+        -> Result<Option<(Size, Primitive, u128)>, LayoutError<'tcx>>
         where C: LayoutOf<Ty<'tcx>, TyLayout = Result<Self, LayoutError<'tcx>>> +
                  HasTyCtxt<'tcx>
     {
         let tcx = cx.tcx();
         match (&self.variants, self.abi, &self.ty.sty) {
             // FIXME(eddyb) check this via value ranges on scalars.
+            (_, Abi::Scalar(Int(I1, _)), _) => {
+                Ok(Some((Size::from_bytes(0), Int(I8, false), 2)))
+            }
+            (_, Abi::Scalar(Int(I32, _)), &ty::TyChar) => {
+                Ok(Some((Size::from_bytes(0), Int(I32, false), 0x10FFFF+1)))
+            }
             (_, Abi::Scalar(Pointer), &ty::TyRef(..)) |
             (_, Abi::Scalar(Pointer), &ty::TyFnPtr(..)) => {
-                Ok(Some((Size::from_bytes(0), Pointer)))
+                Ok(Some((Size::from_bytes(0), Pointer, 0)))
             }
             (_, Abi::Scalar(Pointer), &ty::TyAdt(def, _)) if def.is_box() => {
-                Ok(Some((Size::from_bytes(0), Pointer)))
+                Ok(Some((Size::from_bytes(0), Pointer, 0)))
             }
 
             // FIXME(eddyb) check this via value ranges on scalars.
-            (&Variants::Tagged { discr, .. }, _, &ty::TyAdt(def, _)) => {
+            (&Variants::Tagged { discr, ref discr_range, .. }, _, _) => {
-                if def.discriminants(tcx).all(|d| d.to_u128_unchecked() != 0) {
+                // FIXME(eddyb) support negative/wrap-around discriminant ranges.
-                    Ok(Some((self.fields.offset(0), discr)))
+                if discr_range.start < discr_range.end {
+                    if discr_range.start > 0 {
+                        Ok(Some((self.fields.offset(0), discr, 0)))
+                    } else {
+                        let bits = discr.size(tcx).bits();
+                        assert!(bits <= 128);
+                        let max_value = !0u128 >> (128 - bits);
+                        if discr_range.end < max_value {
+                            Ok(Some((self.fields.offset(0), discr, discr_range.end + 1)))
+                        } else {
+                            Ok(None)
+                        }
+                    }
                 } else {
                     Ok(None)
                 }
@@ -2118,7 +2137,7 @@ impl<'a, 'tcx> TyLayout<'tcx> {
                 let field = self.field(cx, 0)?;
                 let offset = self.fields.offset(0);
                 if let Abi::Scalar(value) = field.abi {
-                    Ok(Some((offset, value)))
+                    Ok(Some((offset, value, 0)))
                 } else {
                     Ok(None)
                 }
@@ -2128,13 +2147,14 @@ impl<'a, 'tcx> TyLayout<'tcx> {
             _ => {
                 if let FieldPlacement::Array { count, .. } = self.fields {
                     if count > 0 {
-                        return self.field(cx, 0)?.non_zero_field(cx);
+                        return self.field(cx, 0)?.find_niche(cx);
                     }
                 }
                 for i in 0..self.fields.count() {
-                    let r = self.field(cx, i)?.non_zero_field(cx)?;
+                    let r = self.field(cx, i)?.find_niche(cx)?;
-                    if let Some((offset, primitive)) = r {
+                    if let Some((offset, primitive, niche_value)) = r {
-                        return Ok(Some((self.fields.offset(i) + offset, primitive)));
+                        let offset = self.fields.offset(i) + offset;
+                        return Ok(Some((offset, primitive, niche_value)));
                     }
                 }
                 Ok(None)
@@ -2165,13 +2185,15 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for Variants {
                 variants.hash_stable(hcx, hasher);
             }
             NicheFilling {
-                nndiscr,
+                dataful_variant,
+                ref niche,
+                niche_value,
                 ref variants,
-                ref discr,
             } => {
-                nndiscr.hash_stable(hcx, hasher);
+                dataful_variant.hash_stable(hcx, hasher);
+                niche.hash_stable(hcx, hasher);
+                niche_value.hash_stable(hcx, hasher);
                 variants.hash_stable(hcx, hasher);
-                discr.hash_stable(hcx, hasher);
             }
         }
     }

src/librustc_trans/debuginfo/metadata.rs

@@ -1191,17 +1191,13 @@ impl<'tcx> EnumMemberDescriptionFactory<'tcx> {
                     }
                 }).collect()
             }
-            layout::Variants::NicheFilling {
+            layout::Variants::NicheFilling { dataful_variant, .. } => {
-                nndiscr,
+                let variant = self.layout.for_variant(dataful_variant);
-                discr,
-                ..
-            } => {
-                let variant = self.layout.for_variant(nndiscr as usize);
                 // Create a description of the non-null variant
                 let (variant_type_metadata, member_description_factory) =
                     describe_enum_variant(cx,
                                           variant,
-                                          &adt.variants[nndiscr as usize],
+                                          &adt.variants[dataful_variant],
                                           OptimizedDiscriminant,
                                           self.containing_scope,
                                           self.span);
@@ -1239,8 +1235,8 @@ impl<'tcx> EnumMemberDescriptionFactory<'tcx> {
                 compute_field_path(cx, &mut name,
                                    self.layout,
                                    self.layout.fields.offset(0),
-                                   discr.size(cx));
+                                   self.layout.field(cx, 0).size);
-                name.push_str(&adt.variants[(1 - nndiscr) as usize].name.as_str());
+                name.push_str(&adt.variants[1 - dataful_variant].name.as_str());
 
                 // Create the (singleton) list of descriptions of union members.
                 vec![

src/librustc_trans/mir/constant.rs

@@ -1122,22 +1122,29 @@ fn trans_const_adt<'a, 'tcx>(
                 },
                 _ => 0,
             };
-            let discr_ty = l.field(ccx, 0).ty;
+            let discr_field = l.field(ccx, 0);
-            let discr = C_int(ccx.layout_of(discr_ty).llvm_type(ccx), discr as i64);
+            let discr = C_int(discr_field.llvm_type(ccx), discr as i64);
             if let layout::Abi::Scalar(_) = l.abi {
                 Const::new(discr, t)
             } else {
-                let discr = Const::new(discr, discr_ty);
+                let discr = Const::new(discr, discr_field.ty);
                 build_const_struct(ccx, l.for_variant(variant_index), vals, Some(discr))
             }
         }
-        layout::Variants::NicheFilling { nndiscr, .. } => {
+        layout::Variants::NicheFilling { dataful_variant, niche_value, .. } => {
-            if variant_index as u64 == nndiscr {
+            if variant_index == dataful_variant {
-                build_const_struct(ccx, l.for_variant(variant_index), vals, None)
+                build_const_struct(ccx, l.for_variant(dataful_variant), vals, None)
             } else {
-                // Always use null even if it's not the `discrfield`th
+                let niche = l.field(ccx, 0);
-                // field; see #8506.
+                let niche_llty = niche.llvm_type(ccx);
-                Const::new(C_null(ccx.layout_of(t).llvm_type(ccx)), t)
+                // FIXME(eddyb) Check the actual primitive type here.
+                let niche_llval = if niche_value == 0 {
+                    // HACK(eddyb) Using `C_null` as it works on all types.
+                    C_null(niche_llty)
+                } else {
+                    C_uint_big(niche_llty, niche_value)
+                };
+                build_const_struct(ccx, l, &[Const::new(niche_llval, niche.ty)], None)
             }
         }
     }

src/librustc_trans/mir/lvalue.rs

@@ -16,7 +16,7 @@ use rustc::mir::tcx::LvalueTy;
 use rustc_data_structures::indexed_vec::Idx;
 use base;
 use builder::Builder;
-use common::{self, CrateContext, C_usize, C_u8, C_u32, C_uint, C_int, C_null};
+use common::{self, CrateContext, C_usize, C_u8, C_u32, C_uint, C_int, C_null, C_uint_big};
 use consts;
 use type_of::LayoutLlvmExt;
 use type_::Type;
@@ -72,10 +72,6 @@ impl Alignment {
     }
 }
 
-fn target_sets_discr_via_memset<'a, 'tcx>(bcx: &Builder<'a, 'tcx>) -> bool {
-    bcx.sess().target.target.arch == "arm" || bcx.sess().target.target.arch == "aarch64"
-}
-
 #[derive(Copy, Clone, Debug)]
 pub struct LvalueRef<'tcx> {
     /// Pointer to the contents of the lvalue
@@ -325,10 +321,17 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
                 };
                 bcx.intcast(lldiscr, cast_to, signed)
             }
-            layout::Variants::NicheFilling { nndiscr, .. } => {
+            layout::Variants::NicheFilling { dataful_variant, niche_value, .. } => {
-                let cmp = if nndiscr == 0 { llvm::IntEQ } else { llvm::IntNE };
+                let niche_llty = discr.layout.llvm_type(bcx.ccx);
-                let zero = C_null(discr.layout.llvm_type(bcx.ccx));
+                // FIXME(eddyb) Check the actual primitive type here.
-                bcx.intcast(bcx.icmp(cmp, lldiscr, zero), cast_to, false)
+                let niche_llval = if niche_value == 0 {
+                    // HACK(eddyb) Using `C_null` as it works on all types.
+                    C_null(niche_llty)
+                } else {
+                    C_uint_big(niche_llty, niche_value)
+                };
+                let cmp = if dataful_variant == 0 { llvm::IntEQ } else { llvm::IntNE };
+                bcx.intcast(bcx.icmp(cmp, lldiscr, niche_llval), cast_to, false)
             }
         }
     }
@@ -336,40 +339,42 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
     /// Set the discriminant for a new value of the given case of the given
     /// representation.
     pub fn trans_set_discr(&self, bcx: &Builder<'a, 'tcx>, variant_index: usize) {
-        let to = self.layout.ty.ty_adt_def().unwrap()
-            .discriminant_for_variant(bcx.tcx(), variant_index)
-            .to_u128_unchecked() as u64;
         match self.layout.variants {
             layout::Variants::Single { index } => {
-                assert_eq!(to, 0);
                 assert_eq!(variant_index, index);
             }
             layout::Variants::Tagged { .. } => {
                 let ptr = self.project_field(bcx, 0);
+                let to = self.layout.ty.ty_adt_def().unwrap()
+                    .discriminant_for_variant(bcx.tcx(), variant_index)
+                    .to_u128_unchecked() as u64;
                 bcx.store(C_int(ptr.layout.llvm_type(bcx.ccx), to as i64),
                     ptr.llval, ptr.alignment.non_abi());
             }
-            layout::Variants::NicheFilling { nndiscr, .. } => {
+            layout::Variants::NicheFilling { dataful_variant, niche_value, .. } => {
-                if to != nndiscr {
+                if variant_index != dataful_variant {
-                    let use_memset = match self.layout.abi {
+                    if bcx.sess().target.target.arch == "arm" ||
-                        layout::Abi::Scalar(_) => false,
+                       bcx.sess().target.target.arch == "aarch64" {
-                        _ => target_sets_discr_via_memset(bcx)
+                        // Issue #34427: As workaround for LLVM bug on ARM,
-                    };
+                        // use memset of 0 before assigning niche value.
-                    if use_memset {
-                        // Issue #34427: As workaround for LLVM bug on
-                        // ARM, use memset of 0 on whole struct rather
-                        // than storing null to single target field.
                         let llptr = bcx.pointercast(self.llval, Type::i8(bcx.ccx).ptr_to());
                         let fill_byte = C_u8(bcx.ccx, 0);
                         let (size, align) = self.layout.size_and_align();
                         let size = C_usize(bcx.ccx, size.bytes());
                         let align = C_u32(bcx.ccx, align.abi() as u32);
                         base::call_memset(bcx, llptr, fill_byte, size, align, false);
-                    } else {
-                        let ptr = self.project_field(bcx, 0);
-                        bcx.store(C_null(ptr.layout.llvm_type(bcx.ccx)),
-                            ptr.llval, ptr.alignment.non_abi());
                     }
+
+                    let niche = self.project_field(bcx, 0);
+                    let niche_llty = niche.layout.llvm_type(bcx.ccx);
+                    // FIXME(eddyb) Check the actual primitive type here.
+                    let niche_llval = if niche_value == 0 {
+                        // HACK(eddyb) Using `C_null` as it works on all types.
+                        C_null(niche_llty)
+                    } else {
+                        C_uint_big(niche_llty, niche_value)
+                    };
+                    bcx.store(niche_llval, niche.llval, niche.alignment.non_abi());
                 }
             }
         }

src/test/ui/print_type_sizes/niche-filling.rs

@@ -10,8 +10,8 @@
 
 // compile-flags: -Z print-type-sizes
 
-// This file illustrates how enums with a non-null field are handled,
+// This file illustrates how niche-filling enums are handled,
-// modelled after cases like `Option<&u32>` and such.
+// modelled after cases like `Option<&u32>`, `Option<bool>` and such.
 //
 // It uses NonZero directly, rather than `&_` or `Unique<_>`, because
 // the test is not set up to deal with target-dependent pointer width.
@@ -72,4 +72,8 @@ pub fn main() {
     let _x: MyOption<NonZero<u32>> = Default::default();
     let _y: EmbeddedDiscr = Default::default();
     let _z: MyOption<IndirectNonZero<u32>> = Default::default();
+    let _a: MyOption<bool> = Default::default();
+    let _b: MyOption<char> = Default::default();
+    let _c: MyOption<std::cmp::Ordering> = Default::default();
+    let _b: MyOption<MyOption<u8>> = Default::default();
 }

src/test/ui/print_type_sizes/niche-filling.stdout

@@ -19,9 +19,35 @@ print-type-size     field `.val`: 4 bytes
 print-type-size     field `.post`: 2 bytes
 print-type-size     field `.pre`: 1 bytes
 print-type-size     end padding: 1 bytes
+print-type-size type: `MyOption<char>`: 4 bytes, alignment: 4 bytes
+print-type-size     variant `None`: 0 bytes
+print-type-size     variant `Some`: 4 bytes
+print-type-size         field `.0`: 4 bytes
 print-type-size type: `MyOption<core::nonzero::NonZero<u32>>`: 4 bytes, alignment: 4 bytes
 print-type-size     variant `None`: 0 bytes
 print-type-size     variant `Some`: 4 bytes
 print-type-size         field `.0`: 4 bytes
 print-type-size type: `core::nonzero::NonZero<u32>`: 4 bytes, alignment: 4 bytes
 print-type-size     field `.0`: 4 bytes
+print-type-size type: `MyOption<MyOption<u8>>`: 2 bytes, alignment: 1 bytes
+print-type-size     variant `None`: 0 bytes
+print-type-size     variant `Some`: 2 bytes
+print-type-size         field `.0`: 2 bytes
+print-type-size type: `MyOption<u8>`: 2 bytes, alignment: 1 bytes
+print-type-size     discriminant: 1 bytes
+print-type-size     variant `None`: 0 bytes
+print-type-size     variant `Some`: 1 bytes
+print-type-size         field `.0`: 1 bytes
+print-type-size type: `MyOption<bool>`: 1 bytes, alignment: 1 bytes
+print-type-size     variant `None`: 0 bytes
+print-type-size     variant `Some`: 1 bytes
+print-type-size         field `.0`: 1 bytes
+print-type-size type: `MyOption<core::cmp::Ordering>`: 1 bytes, alignment: 1 bytes
+print-type-size     variant `None`: 0 bytes
+print-type-size     variant `Some`: 1 bytes
+print-type-size         field `.0`: 1 bytes
+print-type-size type: `core::cmp::Ordering`: 1 bytes, alignment: 1 bytes
+print-type-size     discriminant: 1 bytes
+print-type-size     variant `Less`: 0 bytes
+print-type-size     variant `Equal`: 0 bytes
+print-type-size     variant `Greater`: 0 bytes