rustc: do not inject discriminant fields into Layout::General's variants.

src/librustc/ty/layout.rs

@@ -651,14 +651,14 @@ pub struct Struct {
 }
 
 /// Info required to optimize struct layout.
-#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug)]
+#[derive(Copy, Clone, Debug)]
 enum StructKind {
     /// A tuple, closure, or univariant which cannot be coerced to unsized.
     AlwaysSizedUnivariant,
     /// A univariant, the last field of which may be coerced to unsized.
     MaybeUnsizedUnivariant,
     /// A univariant, but part of an enum.
-    EnumVariant,
+    EnumVariant(Integer),
 }
 
 impl<'a, 'tcx> Struct {
@@ -692,30 +692,27 @@ fn new(dl: &TargetDataLayout,
         // Neither do  1-member and 2-member structs.
         // In addition, code in trans assume that 2-element structs can become pairs.
         // It's easier to just short-circuit here.
-        let can_optimize = (fields.len() > 2 || StructKind::EnumVariant == kind)
-            && (repr.flags & ReprFlags::IS_UNOPTIMISABLE).is_empty();
-
-        let (optimize, sort_ascending) = match kind {
-            StructKind::AlwaysSizedUnivariant => (can_optimize, false),
-            StructKind::MaybeUnsizedUnivariant => (can_optimize, false),
-            StructKind::EnumVariant => {
-                assert!(fields.len() >= 1, "Enum variants must have discriminants.");
-                (can_optimize && fields[0].size(dl).bytes() == 1, true)
+        let (mut optimize, sort_ascending) = match kind {
+            StructKind::AlwaysSizedUnivariant |
+            StructKind::MaybeUnsizedUnivariant => (fields.len() > 2, false),
+            StructKind::EnumVariant(discr) => {
+                (discr.size().bytes() == 1, true)
             }
         };
 
+        optimize &= (repr.flags & ReprFlags::IS_UNOPTIMISABLE).is_empty();
+
         ret.offsets = vec![Size::from_bytes(0); fields.len()];
         let mut inverse_memory_index: Vec<u32> = (0..fields.len() as u32).collect();
 
         if optimize {
-            let start = if let StructKind::EnumVariant = kind { 1 } else { 0 };
             let end = if let StructKind::MaybeUnsizedUnivariant = kind {
                 fields.len() - 1
             } else {
                 fields.len()
             };
-            if end > start {
-                let optimizing  = &mut inverse_memory_index[start..end];
+            if end > 0 {
+                let optimizing  = &mut inverse_memory_index[..end];
                 if sort_ascending {
                     optimizing.sort_by_key(|&x| fields[x as usize].align(dl).abi());
                 } else {
@@ -734,13 +731,17 @@ fn new(dl: &TargetDataLayout,
         // field 5 with offset 0 puts 0 in offsets[5].
         // At the bottom of this function, we use inverse_memory_index to produce memory_index.
 
-        if let StructKind::EnumVariant = kind {
-            assert_eq!(inverse_memory_index[0], 0,
-              "Enum variant discriminants must have the lowest offset.");
-        }
-
         let mut offset = Size::from_bytes(0);
 
+        if let StructKind::EnumVariant(discr) = kind {
+            offset = discr.size();
+            if !ret.packed {
+                let align = discr.align(dl);
+                ret.align = ret.align.max(align);
+                ret.primitive_align = ret.primitive_align.max(align);
+            }
+        }
+
         for i in inverse_memory_index.iter() {
             let field = fields[*i as usize];
             if !ret.sized {
@@ -1112,8 +1113,9 @@ pub enum Layout {
         variants: Union,
     },
 
-    /// General-case enums: for each case there is a struct, and they
-    /// all start with a field for the discriminant.
+    /// General-case enums: for each case there is a struct, and they all have
+    /// all space reserved for the discriminant, and their first field starts
+    /// at a non-0 offset, after where the discriminant would go.
     General {
         discr: Integer,
         variants: Vec<Struct>,
@@ -1495,21 +1497,17 @@ pub fn compute_uncached(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                 // We're interested in the smallest alignment, so start large.
                 let mut start_align = Align::from_bytes(256, 256).unwrap();
 
-                // Create the set of structs that represent each variant
-                // Use the minimum integer type we figured out above
-                let discr = Scalar { value: Int(min_ity), non_zero: false };
+                // Create the set of structs that represent each variant.
                 let mut variants = variants.into_iter().map(|fields| {
-                    let mut fields = fields.into_iter().map(|field| {
+                    let fields = fields.into_iter().map(|field| {
                         field.layout(tcx, param_env)
                     }).collect::<Result<Vec<_>, _>>()?;
-                    fields.insert(0, &discr);
                     let st = Struct::new(dl,
                         &fields,
-                        &def.repr, StructKind::EnumVariant, ty)?;
+                        &def.repr, StructKind::EnumVariant(min_ity), ty)?;
                     // Find the first field we can't move later
                     // to make room for a larger discriminant.
-                    // It is important to skip the first field.
-                    for i in st.field_index_by_increasing_offset().skip(1) {
+                    for i in st.field_index_by_increasing_offset() {
                         let field = fields[i];
                         let field_align = field.align(dl);
                         if field.size(dl).bytes() != 0 || field_align.abi() != 1 {
@@ -1569,9 +1567,8 @@ pub fn compute_uncached(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                     let new_ity_size = Int(ity).size(dl);
                     for variant in &mut variants {
                         for i in variant.offsets.iter_mut() {
-                            // The first field is the discrimminant, at offset 0.
-                            // These aren't in order, and we need to skip it.
-                            if *i <= old_ity_size && *i > Size::from_bytes(0) {
+                            if *i <= old_ity_size {
+                                assert_eq!(*i, old_ity_size);
                                 *i = new_ity_size;
                             }
                         }
@@ -1759,7 +1756,7 @@ pub fn field_offset<C: HasDataLayout>(&self,
 
             General { ref variants, .. } => {
                 let v = variant_index.expect("variant index required");
-                variants[v].offsets[i + 1]
+                variants[v].offsets[i]
             }
 
             StructWrappedNullablePointer { nndiscr, ref nonnull, .. } => {
@@ -1857,21 +1854,12 @@ fn record_layout_for_printing_outlined(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             }
         };
 
-        enum Fields<'a> {
-            WithDiscrim(&'a Struct),
-            NoDiscrim(&'a Struct),
-        }
-
         let build_variant_info = |n: Option<ast::Name>,
                                   flds: &[(ast::Name, Ty<'tcx>)],
-                                  layout: Fields| {
-            let (s, field_offsets) = match layout {
-                Fields::WithDiscrim(s) => (s, &s.offsets[1..]),
-                Fields::NoDiscrim(s) => (s, &s.offsets[0..]),
-            };
+                                  s: &Struct| {
             let field_info: Vec<_> =
                 flds.iter()
-                    .zip(field_offsets.iter())
+                    .zip(&s.offsets)
                     .map(|(&field_name_ty, offset)| build_field_info(field_name_ty, offset))
                     .collect();
 
@@ -1904,7 +1892,7 @@ enum Fields<'a> {
                        None,
                        vec![build_variant_info(Some(variant_def.name),
                                                &fields,
-                                               Fields::NoDiscrim(variant_layout))]);
+                                               variant_layout)]);
             }
             Layout::RawNullablePointer { nndiscr, value } => {
                 debug!("print-type-size t: `{:?}` adt raw nullable nndiscr {} is {:?}",
@@ -1931,7 +1919,7 @@ enum Fields<'a> {
                            None,
                            vec![build_variant_info(Some(variant_def.name),
                                                    &fields,
-                                                   Fields::NoDiscrim(variant_layout))]);
+                                                   variant_layout)]);
                 } else {
                     // (This case arises for *empty* enums; so give it
                     // zero variants.)
@@ -1953,7 +1941,7 @@ enum Fields<'a> {
                                                        .collect();
                                         build_variant_info(Some(variant_def.name),
                                                            &fields,
-                                                           Fields::WithDiscrim(variant_layout))
+                                                           variant_layout)
                                     })
                                     .collect();
                 record(adt_kind.into(), Some(discr.size()), variant_infos);

src/librustc_trans/adt.rs

@@ -90,8 +90,7 @@ pub fn finish_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                     (l.for_variant(nndiscr as usize), nonnull),
                 _ => unreachable!()
             };
-            llty.set_struct_body(&struct_llfields(cx, variant_layout, variant, None),
-                                 variant.packed)
+            llty.set_struct_body(&struct_llfields(cx, variant_layout, variant), variant.packed)
         },
         _ => bug!("This function cannot handle {} with layout {:#?}", t, l)
     }
@@ -116,7 +115,7 @@ fn generic_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
             match name {
                 None => {
                     Type::struct_(cx, &struct_llfields(cx, l.for_variant(nndiscr as usize),
-                                                       nonnull, None),
+                                                       nonnull),
                                   nonnull.packed)
                 }
                 Some(name) => {
@@ -127,7 +126,7 @@ fn generic_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
         layout::Univariant { ref variant, .. } => {
             match name {
                 None => {
-                    Type::struct_(cx, &struct_llfields(cx, l, &variant, None),
+                    Type::struct_(cx, &struct_llfields(cx, l, &variant),
                                   variant.packed)
                 }
                 Some(name) => {
@@ -209,23 +208,16 @@ pub fn memory_index_to_gep(index: u64) -> u64 {
 
 pub fn struct_llfields<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                                  layout: TyLayout<'tcx>,
-                                 variant: &layout::Struct,
-                                 discr: Option<Ty<'tcx>>) -> Vec<Type> {
-    let field_count = (discr.is_some() as usize) + layout.field_count();
+                                 variant: &layout::Struct) -> Vec<Type> {
+    let field_count = layout.field_count();
     debug!("struct_llfields: variant: {:?}", variant);
     let mut offset = Size::from_bytes(0);
     let mut result: Vec<Type> = Vec::with_capacity(1 + field_count * 2);
-    let field_iter = variant.field_index_by_increasing_offset().map(|i| {
-        let ty = if i == 0 && discr.is_some() {
-            cx.layout_of(discr.unwrap())
-        } else {
-            layout.field(cx, i - discr.is_some() as usize)
-        };
-        (i, ty, variant.offsets[i as usize])
-    });
-    for (index, field, target_offset) in field_iter {
+    for i in variant.field_index_by_increasing_offset() {
+        let field = layout.field(cx, i);
+        let target_offset = variant.offsets[i as usize];
         debug!("struct_llfields: {}: {:?} offset: {:?} target_offset: {:?}",
-            index, field, offset, target_offset);
+            i, field, offset, target_offset);
         assert!(target_offset >= offset);
         let padding = target_offset - offset;
         result.push(Type::array(&Type::i8(cx), padding.bytes()));

src/librustc_trans/debuginfo/metadata.rs

@@ -1340,7 +1340,7 @@ fn create_member_descriptions<'a>(&self, cx: &CrateContext<'a, 'tcx>)
 // Creates MemberDescriptions for the fields of a single enum variant.
 struct VariantMemberDescriptionFactory<'tcx> {
     // Cloned from the layout::Struct describing the variant.
-    offsets: &'tcx [layout::Size],
+    offsets: Vec<layout::Size>,
     args: Vec<(String, Ty<'tcx>)>,
     discriminant_type_metadata: Option<DIType>,
     span: Span,
@@ -1436,8 +1436,12 @@ fn describe_enum_variant<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
     };
 
     // If this is not a univariant enum, there is also the discriminant field.
+    let mut offsets = struct_def.offsets.clone();
     match discriminant_info {
-        RegularDiscriminant(_) => arg_names.insert(0, "RUST$ENUM$DISR".to_string()),
+        RegularDiscriminant(_) => {
+            arg_names.insert(0, "RUST$ENUM$DISR".to_string());
+            offsets.insert(0, Size::from_bytes(0));
+        }
         _ => { /* do nothing */ }
     };
 
@@ -1449,7 +1453,7 @@ fn describe_enum_variant<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
 
     let member_description_factory =
         VariantMDF(VariantMemberDescriptionFactory {
-            offsets: &struct_def.offsets[..],
+            offsets,
             args,
             discriminant_type_metadata: match discriminant_info {
                 RegularDiscriminant(discriminant_type_metadata) => {

src/librustc_trans/mir/constant.rs

@@ -1108,11 +1108,8 @@ fn trans_const_adt<'a, 'tcx>(
         layout::General { discr: d, ref variants, .. } => {
             let variant = &variants[variant_index];
             let lldiscr = C_int(Type::from_integer(ccx, d), variant_index as i64);
-            let mut vals_with_discr = vec![
-                Const::new(lldiscr, d.to_ty(ccx.tcx(), false))
-            ];
-            vals_with_discr.extend_from_slice(vals);
-            build_const_struct(ccx, l, &variant, &vals_with_discr)
+            build_const_struct(ccx, l, &variant, vals,
+                Some(Const::new(lldiscr, d.to_ty(ccx.tcx(), false))))
         }
         layout::UntaggedUnion { ref variants, .. }=> {
             assert_eq!(variant_index, 0);
@@ -1125,7 +1122,7 @@ fn trans_const_adt<'a, 'tcx>(
         }
         layout::Univariant { ref variant, .. } => {
             assert_eq!(variant_index, 0);
-            build_const_struct(ccx, l, &variant, vals)
+            build_const_struct(ccx, l, &variant, vals, None)
         }
         layout::Vector { .. } => {
             Const::new(C_vector(&vals.iter().map(|x| x.llval).collect::<Vec<_>>()), t)
@@ -1140,7 +1137,7 @@ fn trans_const_adt<'a, 'tcx>(
         }
         layout::StructWrappedNullablePointer { ref nonnull, nndiscr, .. } => {
             if variant_index as u64 == nndiscr {
-                build_const_struct(ccx, l, &nonnull, vals)
+                build_const_struct(ccx, l, &nonnull, vals, None)
             } else {
                 // Always use null even if it's not the `discrfield`th
                 // field; see #8506.
@@ -1162,14 +1159,20 @@ fn trans_const_adt<'a, 'tcx>(
 fn build_const_struct<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
                                 layout: layout::TyLayout<'tcx>,
                                 st: &layout::Struct,
-                                vals: &[Const<'tcx>])
+                                vals: &[Const<'tcx>],
+                                discr: Option<Const<'tcx>>)
                                 -> Const<'tcx> {
     assert_eq!(vals.len(), st.offsets.len());
 
     // offset of current value
     let mut offset = Size::from_bytes(0);
     let mut cfields = Vec::new();
-    cfields.reserve(st.offsets.len()*2);
+    cfields.reserve(discr.is_some() as usize + 1 + st.offsets.len() * 2);
+
+    if let Some(discr) = discr {
+        cfields.push(discr.llval);
+        offset = ccx.size_of(discr.ty);
+    }
 
     let parts = st.field_index_by_increasing_offset().map(|i| {
         (vals[i], st.offsets[i])

src/librustc_trans/mir/lvalue.rs

@@ -236,14 +236,6 @@ pub fn project_field(self, bcx: &Builder<'a, 'tcx>, ix: usize) -> LvalueRef<'tcx
             _ => {}
         }
 
-        // Adjust the index to account for enum discriminants in variants.
-        let mut ix = ix;
-        if let layout::General { .. } = *l {
-            if l.variant_index.is_some() {
-                ix += 1;
-            }
-        }
-
         let simple = || {
             LvalueRef {
                 llval: bcx.struct_gep(self.llval, l.llvm_field_index(ix)),
@@ -474,11 +466,10 @@ pub fn project_downcast(&self, bcx: &Builder<'a, 'tcx>, variant_index: usize)
 
             // If this is an enum, cast to the appropriate variant struct type.
             let layout = bcx.ccx.layout_of(ty).for_variant(variant_index);
-            if let layout::General { discr, ref variants, .. } = *layout {
+            if let layout::General { ref variants, .. } = *layout {
                 let st = &variants[variant_index];
                 let variant_ty = Type::struct_(bcx.ccx,
-                    &adt::struct_llfields(bcx.ccx, layout, st,
-                        Some(discr.to_ty(bcx.tcx(), false))), st.packed);
+                    &adt::struct_llfields(bcx.ccx, layout, st), st.packed);
                 downcast.llval = bcx.pointercast(downcast.llval, variant_ty.ptr_to());
             }
 

src/test/codegen/consts.rs

@@ -54,7 +54,7 @@ pub fn inline_enum_const() -> E<i8, i16> {
 #[no_mangle]
 pub fn low_align_const() -> E<i16, [i16; 3]> {
 // Check that low_align_const and high_align_const use the same constant
-// CHECK: load {{.*}} bitcast ({ [0 x i8], i16, [0 x i8], i16, [4 x i8] }** [[LOW_HIGH_REF]]
+// CHECK: load {{.*}} bitcast ({ i16, [0 x i8], i16, [4 x i8] }** [[LOW_HIGH_REF]]
     *&E::A(0)
 }
 
@@ -62,6 +62,6 @@ pub fn inline_enum_const() -> E<i8, i16> {
 #[no_mangle]
 pub fn high_align_const() -> E<i16, i32> {
 // Check that low_align_const and high_align_const use the same constant
-// CHECK: load {{.*}} bitcast ({ [0 x i8], i16, [0 x i8], i16, [4 x i8] }** [[LOW_HIGH_REF]]
+// CHECK: load {{.*}} bitcast ({ i16, [0 x i8], i16, [4 x i8] }** [[LOW_HIGH_REF]]
     *&E::A(0)
 }

src/test/run-pass/enum-discrim-manual-sizing.rs

@@ -108,6 +108,9 @@ pub fn main() {
     let array_expected_size = round_up(28, align_of::<Eu64NonCLike<[u32; 5]>>());
     assert_eq!(size_of::<Eu64NonCLike<[u32; 5]>>(), array_expected_size);
     assert_eq!(size_of::<Eu64NonCLike<[u32; 6]>>(), 32);
+
+    assert_eq!(align_of::<Eu32>(), align_of::<u32>());
+    assert_eq!(align_of::<Eu64NonCLike<u8>>(), align_of::<u64>());
 }
 
 // Rounds x up to the next multiple of a