rustc: use an offset instead of a field path in Layout::StructWrappedNullablePointer.

6 files changed, 137 insertions, 113 deletions

diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs
index a7c707de481..c16092666f7 100644
--- a/src/librustc/ty/layout.rs
+++ b/src/librustc/ty/layout.rs
@@ -841,35 +841,47 @@ impl<'a, 'tcx> Struct {
         })
     }
 
-    /// Find the path leading to a non-zero leaf field, starting from
+    /// Find the offset of a non-zero leaf field, starting from
     /// the given type and recursing through aggregates.
-    /// The tuple is `(path, source_path)`,
-    /// where `path` is in memory order and `source_path` in source order.
+    /// The tuple is `(offset, primitive, source_path)`.
     // FIXME(eddyb) track value ranges and traverse already optimized enums.
     fn non_zero_field_in_type(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                               param_env: ty::ParamEnv<'tcx>,
                               ty: Ty<'tcx>)
-                              -> Result<Option<(FieldPath, FieldPath)>, LayoutError<'tcx>> {
-        match (ty.layout(tcx, param_env)?, &ty.sty) {
-            (&Scalar { non_zero: true, .. }, _) |
-            (&CEnum { non_zero: true, .. }, _) => Ok(Some((vec![], vec![]))),
+                              -> Result<Option<(Size, Primitive, FieldPath)>, LayoutError<'tcx>> {
+        let layout = ty.layout(tcx, param_env)?;
+        match (layout, &ty.sty) {
+            (&Scalar { non_zero: true, value, .. }, _) => {
+                Ok(Some((Size::from_bytes(0), value, vec![])))
+            }
+            (&CEnum { non_zero: true, discr, .. }, _) => {
+                Ok(Some((Size::from_bytes(0), Int(discr), vec![])))
+            }
+
             (&FatPointer { non_zero: true, .. }, _) => {
-                Ok(Some((vec![FAT_PTR_ADDR as u32], vec![FAT_PTR_ADDR as u32])))
+                Ok(Some((layout.field_offset(tcx, FAT_PTR_ADDR, None),
+                         Pointer,
+                         vec![FAT_PTR_ADDR as u32])))
             }
 
             // Is this the NonZero lang item wrapping a pointer or integer type?
             (&Univariant { non_zero: true, .. }, &ty::TyAdt(def, substs)) => {
                 let fields = &def.struct_variant().fields;
                 assert_eq!(fields.len(), 1);
-                match *fields[0].ty(tcx, substs).layout(tcx, param_env)? {
+                let field = fields[0].ty(tcx, substs).layout(tcx, param_env)?;
+                match *field {
                     // FIXME(eddyb) also allow floating-point types here.
-                    Scalar { value: Int(_), non_zero: false } |
-                    Scalar { value: Pointer, non_zero: false } => {
-                        Ok(Some((vec![0], vec![0])))
+                    Scalar { value: value @ Int(_), non_zero: false } |
+                    Scalar { value: value @ Pointer, non_zero: false } => {
+                        Ok(Some((layout.field_offset(tcx, 0, None),
+                                 value,
+                                 vec![0])))
                     }
                     FatPointer { non_zero: false, .. } => {
-                        let tmp = vec![FAT_PTR_ADDR as u32, 0];
-                        Ok(Some((tmp.clone(), tmp)))
+                        Ok(Some((layout.field_offset(tcx, 0, None) +
+                                 field.field_offset(tcx, FAT_PTR_ADDR, None),
+                                 Pointer,
+                                 vec![FAT_PTR_ADDR as u32, 0])))
                     }
                     _ => Ok(None)
                 }
@@ -878,31 +890,31 @@ impl<'a, 'tcx> Struct {
             // Perhaps one of the fields of this struct is non-zero
             // let's recurse and find out
             (&Univariant { ref variant, .. }, &ty::TyAdt(def, substs)) if def.is_struct() => {
-                Struct::non_zero_field_paths(
+                Struct::non_zero_field(
                     tcx,
                     param_env,
                     def.struct_variant().fields.iter().map(|field| {
                         field.ty(tcx, substs)
                     }),
-                    Some(&variant.memory_index[..]))
+                    &variant.offsets)
             }
 
             // Perhaps one of the upvars of this closure is non-zero
             (&Univariant { ref variant, .. }, &ty::TyClosure(def, substs)) => {
                 let upvar_tys = substs.upvar_tys(def, tcx);
-                Struct::non_zero_field_paths(
+                Struct::non_zero_field(
                     tcx,
                     param_env,
                     upvar_tys,
-                    Some(&variant.memory_index[..]))
+                    &variant.offsets)
             }
             // Can we use one of the fields in this tuple?
             (&Univariant { ref variant, .. }, &ty::TyTuple(tys, _)) => {
-                Struct::non_zero_field_paths(
+                Struct::non_zero_field(
                     tcx,
                     param_env,
                     tys.iter().cloned(),
-                    Some(&variant.memory_index[..]))
+                    &variant.offsets)
             }
 
             // Is this a fixed-size array of something non-zero
@@ -915,11 +927,11 @@ impl<'a, 'tcx> Struct {
                     }
                 }
                 if count.val.to_const_int().unwrap().to_u64().unwrap() != 0 {
-                    Struct::non_zero_field_paths(
+                    Struct::non_zero_field(
                         tcx,
                         param_env,
                         Some(ety).into_iter(),
-                        None)
+                        &[Size::from_bytes(0)])
                 } else {
                     Ok(None)
                 }
@@ -938,27 +950,20 @@ impl<'a, 'tcx> Struct {
         }
     }
 
-    /// Find the path leading to a non-zero leaf field, starting from
+    /// Find the offset of a non-zero leaf field, starting from
     /// the given set of fields and recursing through aggregates.
-    /// Returns Some((path, source_path)) on success.
-    /// `path` is translated to memory order. `source_path` is not.
-    fn non_zero_field_paths<I>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                               param_env: ty::ParamEnv<'tcx>,
-                               fields: I,
-                               permutation: Option<&[u32]>)
-                               -> Result<Option<(FieldPath, FieldPath)>, LayoutError<'tcx>>
+    /// Returns Some((offset, primitive, source_path)) on success.
+    fn non_zero_field<I>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                         param_env: ty::ParamEnv<'tcx>,
+                         fields: I,
+                         offsets: &[Size])
+                         -> Result<Option<(Size, Primitive, FieldPath)>, LayoutError<'tcx>>
     where I: Iterator<Item=Ty<'tcx>> {
         for (i, ty) in fields.enumerate() {
             let r = Struct::non_zero_field_in_type(tcx, param_env, ty)?;
-            if let Some((mut path, mut source_path)) = r {
+            if let Some((offset, primitive, mut source_path)) = r {
                 source_path.push(i as u32);
-                let index = if let Some(p) = permutation {
-                    p[i] as usize
-                } else {
-                    i
-                };
-                path.push(index as u32);
-                return Ok(Some((path, source_path)));
+                return Ok(Some((offsets[i] + offset, primitive, source_path)));
             }
         }
         Ok(None)
@@ -1135,18 +1140,19 @@ pub enum Layout {
     /// identity function.
     RawNullablePointer {
         nndiscr: u64,
-        value: Primitive
+        discr: Primitive
     },
 
     /// Two cases distinguished by a nullable pointer: the case with discriminant
-    /// `nndiscr` is represented by the struct `nonnull`, where the `discrfield`th
-    /// field is known to be nonnull due to its type; if that field is null, then
+    /// `nndiscr` is represented by the struct `nonnull`, where the field at the
+    /// `discr_offset` offset is known to be nonnull due to its type; if that field is null, then
     /// it represents the other case, which is known to be zero sized.
     StructWrappedNullablePointer {
         nndiscr: u64,
         nonnull: Struct,
-        discrfield: FieldPath,
-        /// Like discrfield, but in source order. For debuginfo.
+        discr: Primitive,
+        discr_offset: Size,
+        /// Like discr_offset, but the source field path. For debuginfo.
         discrfield_source: FieldPath
     }
 }
@@ -1440,44 +1446,36 @@ impl<'a, 'tcx> Layout {
                         if !Struct::would_be_zero_sized(dl, other_fields)? {
                             continue;
                         }
-                        let paths = Struct::non_zero_field_paths(tcx,
-                                                                 param_env,
-                                                                 variants[discr].iter().cloned(),
-                                                                 None)?;
-                        let (mut path, mut path_source) = if let Some(p) = paths { p }
+
+                        let st = Struct::new(dl,
+                            &variants[discr].iter().map(|ty| ty.layout(tcx, param_env))
+                              .collect::<Result<Vec<_>, _>>()?,
+                            &def.repr, StructKind::AlwaysSizedUnivariant, ty)?;
+
+                        let field = Struct::non_zero_field(tcx,
+                                                           param_env,
+                                                           variants[discr].iter().cloned(),
+                                                           &st.offsets)?;
+                        let (offset, primitive, mut path_source) = if let Some(f) = field { f }
                           else { continue };
 
                         // FIXME(eddyb) should take advantage of a newtype.
-                        if path == &[0] && variants[discr].len() == 1 {
-                            let value = match *variants[discr][0].layout(tcx, param_env)? {
-                                Scalar { value, .. } => value,
-                                CEnum { discr, .. } => Int(discr),
-                                _ => bug!("Layout::compute: `{}`'s non-zero \
-                                           `{}` field not scalar?!",
-                                           ty, variants[discr][0])
-                            };
+                        if offset.bytes() == 0 && primitive.size(dl) == st.stride() &&
+                           variants[discr].len() == 1 {
                             return success(RawNullablePointer {
                                 nndiscr: discr as u64,
-                                value,
+                                discr: primitive,
                             });
                         }
 
-                        let st = Struct::new(dl,
-                            &variants[discr].iter().map(|ty| ty.layout(tcx, param_env))
-                              .collect::<Result<Vec<_>, _>>()?,
-                            &def.repr, StructKind::AlwaysSizedUnivariant, ty)?;
-
-                        // We have to fix the last element of path here.
-                        let mut i = *path.last().unwrap();
-                        i = st.memory_index[i as usize];
-                        *path.last_mut().unwrap() = i;
-                        path.reverse();
+                        // We have to fix the source path here.
                         path_source.reverse();
 
                         return success(StructWrappedNullablePointer {
                             nndiscr: discr as u64,
                             nonnull: st,
-                            discrfield: path,
+                            discr: primitive,
+                            discr_offset: offset,
                             discrfield_source: path_source
                         });
                     }
@@ -1621,7 +1619,7 @@ impl<'a, 'tcx> Layout {
         let dl = cx.data_layout();
 
         match *self {
-            Scalar { value, .. } | RawNullablePointer { value, .. } => {
+            Scalar { value, .. } | RawNullablePointer { discr: value, .. } => {
                 value.size(dl)
             }
 
@@ -1664,7 +1662,7 @@ impl<'a, 'tcx> Layout {
         let dl = cx.data_layout();
 
         match *self {
-            Scalar { value, .. } | RawNullablePointer { value, .. } => {
+            Scalar { value, .. } | RawNullablePointer { discr: value, .. } => {
                 value.align(dl)
             }
 
@@ -1876,7 +1874,8 @@ impl<'a, 'tcx> Layout {
         match *layout {
             Layout::StructWrappedNullablePointer { nonnull: ref variant_layout,
                                                    nndiscr,
-                                                   discrfield: _,
+                                                   discr: _,
+                                                   discr_offset: _,
                                                    discrfield_source: _ } => {
                 debug!("print-type-size t: `{:?}` adt struct-wrapped nullable nndiscr {} is {:?}",
                        ty, nndiscr, variant_layout);
@@ -1891,12 +1890,12 @@ impl<'a, 'tcx> Layout {
                                                &fields,
                                                variant_layout)]);
             }
-            Layout::RawNullablePointer { nndiscr, value } => {
+            Layout::RawNullablePointer { nndiscr, discr } => {
                 debug!("print-type-size t: `{:?}` adt raw nullable nndiscr {} is {:?}",
-                       ty, nndiscr, value);
+                       ty, nndiscr, discr);
                 let variant_def = &adt_def.variants[nndiscr as usize];
                 record(adt_kind.into(), None,
-                       vec![build_primitive_info(variant_def.name, &value)]);
+                       vec![build_primitive_info(variant_def.name, &discr)]);
             }
             Layout::Univariant { variant: ref variant_layout, non_zero: _ } => {
                 let variant_names = || {
@@ -2410,19 +2409,21 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for Layout
                 align.hash_stable(hcx, hasher);
                 primitive_align.hash_stable(hcx, hasher);
             }
-            RawNullablePointer { nndiscr, ref value } => {
+            RawNullablePointer { nndiscr, ref discr } => {
                 nndiscr.hash_stable(hcx, hasher);
-                value.hash_stable(hcx, hasher);
+                discr.hash_stable(hcx, hasher);
             }
             StructWrappedNullablePointer {
                 nndiscr,
                 ref nonnull,
-                ref discrfield,
+                ref discr,
+                discr_offset,
                 ref discrfield_source
             } => {
                 nndiscr.hash_stable(hcx, hasher);
                 nonnull.hash_stable(hcx, hasher);
-                discrfield.hash_stable(hcx, hasher);
+                discr.hash_stable(hcx, hasher);
+                discr_offset.hash_stable(hcx, hasher);
                 discrfield_source.hash_stable(hcx, hasher);
             }
         }
diff --git a/src/librustc_trans/abi.rs b/src/librustc_trans/abi.rs
index f4f37cdef51..329da2c36a2 100644
--- a/src/librustc_trans/abi.rs
+++ b/src/librustc_trans/abi.rs
@@ -295,7 +295,7 @@ impl<'tcx> LayoutExt<'tcx> for TyLayout<'tcx> {
         match *self.layout {
             // The primitives for this algorithm.
             Layout::Scalar { value, .. } |
-            Layout::RawNullablePointer { value, .. } => {
+            Layout::RawNullablePointer { discr: value, .. } => {
                 let kind = match value {
                     layout::Int(_) |
                     layout::Pointer => RegKind::Integer,
diff --git a/src/librustc_trans/cabi_x86_64.rs b/src/librustc_trans/cabi_x86_64.rs
index 6670d084d6c..dcbb2de9c4d 100644
--- a/src/librustc_trans/cabi_x86_64.rs
+++ b/src/librustc_trans/cabi_x86_64.rs
@@ -66,7 +66,7 @@ fn classify_arg<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, arg: &ArgType<'tcx>)
 
         match *layout {
             Layout::Scalar { value, .. } |
-            Layout::RawNullablePointer { value, .. } => {
+            Layout::RawNullablePointer { discr: value, .. } => {
                 let reg = match value {
                     layout::Int(_) |
                     layout::Pointer => Class::Int,
diff --git a/src/librustc_trans/mir/lvalue.rs b/src/librustc_trans/mir/lvalue.rs
index 7cf35c4405c..7dcb5d219b7 100644
--- a/src/librustc_trans/mir/lvalue.rs
+++ b/src/librustc_trans/mir/lvalue.rs
@@ -10,7 +10,7 @@
 
 use llvm::{self, ValueRef};
 use rustc::ty::{self, Ty, TypeFoldable};
-use rustc::ty::layout::{self, Align, Layout, LayoutTyper};
+use rustc::ty::layout::{self, Align, Layout, LayoutTyper, Size};
 use rustc::mir;
 use rustc::mir::tcx::LvalueTy;
 use rustc_data_structures::indexed_vec::Idx;
@@ -25,7 +25,6 @@ use type_::Type;
 use value::Value;
 use glue;
 
-use std::iter;
 use std::ptr;
 use std::ops;
 
@@ -330,14 +329,26 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
         }
     }
 
-    // Double index to account for padding (FieldPath already uses `Struct::memory_index`)
-    fn gepi_struct_llfields_path(self, bcx: &Builder, discrfield: &layout::FieldPath) -> ValueRef {
-        let path = iter::once(C_u32(bcx.ccx, 0)).chain(discrfield.iter().map(|&i| {
-            let i = adt::memory_index_to_gep(i as u64);
-            assert_eq!(i as u32 as u64, i);
-            C_u32(bcx.ccx, i as u32)
-        })).collect::<Vec<_>>();
-        bcx.inbounds_gep(self.llval, &path)
+    // Return a pointer to the discriminant, given its type and offset.
+    fn gepi_discr_at_offset(self, bcx: &Builder,
+                            discr: ty::layout::Primitive,
+                            offset: Size)
+                            -> (ValueRef, Alignment) {
+        let size = discr.size(bcx.ccx);
+        let ptr_ty = Type::from_primitive(bcx.ccx, discr).ptr_to();
+
+        // If the discriminant is not on a multiple of the primitive's size,
+        // we need to go through i8*. Also assume the worst alignment.
+        if offset.bytes() % size.bytes() != 0 {
+            let byte_ptr = bcx.pointercast(self.llval, Type::i8p(bcx.ccx));
+            let byte_ptr = bcx.inbounds_gep(byte_ptr, &[C_usize(bcx.ccx, offset.bytes())]);
+            let byte_align = Alignment::Packed(Align::from_bytes(1, 1).unwrap());
+            return (bcx.pointercast(byte_ptr, ptr_ty), byte_align);
+        }
+
+        let discr_ptr = bcx.pointercast(self.llval, ptr_ty);
+        (bcx.inbounds_gep(discr_ptr, &[C_usize(bcx.ccx, offset.bytes() / size.bytes())]),
+         self.alignment)
     }
 
     /// Helper for cases where the discriminant is simply loaded.
@@ -378,16 +389,16 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
                 self.load_discr(bcx, discr, ptr.llval, 0, variants.len() as u64 - 1)
             }
             layout::Univariant { .. } | layout::UntaggedUnion { .. } => C_u8(bcx.ccx, 0),
-            layout::RawNullablePointer { nndiscr, .. } => {
-                let cmp = if nndiscr == 0 { llvm::IntEQ } else { llvm::IntNE };
-                let discr = bcx.load(self.llval, self.alignment.non_abi());
-                bcx.icmp(cmp, discr, C_null(val_ty(discr)))
-            }
-            layout::StructWrappedNullablePointer { nndiscr, ref discrfield, .. } => {
-                let llptrptr = self.gepi_struct_llfields_path(bcx, discrfield);
-                let llptr = bcx.load(llptrptr, self.alignment.non_abi());
+            layout::RawNullablePointer { nndiscr, discr } |
+            layout::StructWrappedNullablePointer { nndiscr, discr, .. } => {
+                let discr_offset = match *l {
+                    layout::StructWrappedNullablePointer { discr_offset, .. } => discr_offset,
+                    _ => Size::from_bytes(0),
+                };
+                let (lldiscrptr, alignment) = self.gepi_discr_at_offset(bcx, discr, discr_offset);
+                let lldiscr = bcx.load(lldiscrptr, alignment.non_abi());
                 let cmp = if nndiscr == 0 { llvm::IntEQ } else { llvm::IntNE };
-                bcx.icmp(cmp, llptr, C_null(val_ty(llptr)))
+                bcx.icmp(cmp, lldiscr, C_null(Type::from_primitive(bcx.ccx, discr)))
             },
             _ => bug!("{} is not an enum", l.ty)
         };
@@ -418,27 +429,30 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
             | layout::Vector { .. } => {
                 assert_eq!(to, 0);
             }
-            layout::RawNullablePointer { nndiscr, .. } => {
+            layout::RawNullablePointer { nndiscr, discr, .. } |
+            layout::StructWrappedNullablePointer { nndiscr, discr, .. } => {
                 if to != nndiscr {
-                    let llptrty = val_ty(self.llval).element_type();
-                    bcx.store(C_null(llptrty), self.llval, self.alignment.non_abi());
-                }
-            }
-            layout::StructWrappedNullablePointer { nndiscr, ref discrfield, ref nonnull, .. } => {
-                if to != nndiscr {
-                    if target_sets_discr_via_memset(bcx) {
+                    let (use_memset, discr_offset) = match *l {
+                        layout::StructWrappedNullablePointer { discr_offset, .. } => {
+                            (target_sets_discr_via_memset(bcx), discr_offset)
+                        }
+                        _ => (false, Size::from_bytes(0)),
+                    };
+                    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 = C_usize(bcx.ccx, nonnull.stride().bytes());
-                        let align = C_u32(bcx.ccx, nonnull.align.abi() as u32);
+                        let (size, align) = l.size_and_align(bcx.ccx);
+                        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 llptrptr = self.gepi_struct_llfields_path(bcx, discrfield);
-                        let llptrty = val_ty(llptrptr).element_type();
-                        bcx.store(C_null(llptrty), llptrptr, self.alignment.non_abi());
+                        let (lldiscrptr, alignment) =
+                            self.gepi_discr_at_offset(bcx, discr, discr_offset);
+                        bcx.store(C_null(Type::from_primitive(bcx.ccx, discr)),
+                            lldiscrptr, alignment.non_abi());
                     }
                 }
             }
diff --git a/src/librustc_trans/type_.rs b/src/librustc_trans/type_.rs
index bb8f3f23108..2359aa811fa 100644
--- a/src/librustc_trans/type_.rs
+++ b/src/librustc_trans/type_.rs
@@ -287,4 +287,14 @@ impl Type {
             I128 => Type::i128(cx),
         }
     }
+
+    pub fn from_primitive(cx: &CrateContext, p: layout::Primitive) -> Type {
+        use rustc::ty::layout::Primitive::*;
+        match p {
+            Int(i) => Type::from_integer(cx, i),
+            F32 => Type::f32(cx),
+            F64 => Type::f64(cx),
+            Pointer => Type::i8p(cx),
+        }
+    }
 }
diff --git a/src/test/ui/print_type_sizes/nullable.stdout b/src/test/ui/print_type_sizes/nullable.stdout
index 830678f174f..c9cdde78a4d 100644
--- a/src/test/ui/print_type_sizes/nullable.stdout
+++ b/src/test/ui/print_type_sizes/nullable.stdout
@@ -19,6 +19,5 @@ print-type-size     field `.pre`: 1 bytes
 print-type-size     end padding: 1 bytes
 print-type-size type: `MyOption<core::nonzero::NonZero<u32>>`: 4 bytes, alignment: 4 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