Part of #27023: Put drop flag at end of a type's representation.

This is trickier than it sounds (because the DST code was written
assuming that one could divide the sized and unsized portions of a
type strictly into a sized prefix and unsized suffix, when it reality
it is more like a sized prefix and sized suffix that surround the
single unsized field).

I chose to put in a pretty hack-ish approach to this because
drop-flags are scheduled to go away anyway, so its not really worth
the effort to to make an infrastructure that sounds as general as the
previous paragraph indicates.

Also, I have written notes of other fixes that need to be made to
really finish fixing #27023, namely more work needs to be done to
account for alignment when computing the size of a value.

2 files changed, 128 insertions, 23 deletions

diff --git a/src/librustc_trans/trans/adt.rs b/src/librustc_trans/trans/adt.rs
index b47d2dd4112..1a5eebfa77a 100644
--- a/src/librustc_trans/trans/adt.rs
+++ b/src/librustc_trans/trans/adt.rs
@@ -67,6 +67,31 @@ use trans::type_of;
 
 type Hint = attr::ReprAttr;
 
+// Representation of the context surrounding an unsized type. I want
+// to be able to track the drop flags that are injected by trans.
+#[derive(Clone, Copy, PartialEq, Debug)]
+pub struct TypeContext {
+    prefix: Type,
+    needs_drop_flag: bool,
+}
+
+impl TypeContext {
+    pub fn prefix(&self) -> Type { self.prefix }
+    pub fn needs_drop_flag(&self) -> bool { self.needs_drop_flag }
+
+    fn direct(t: Type) -> TypeContext {
+        TypeContext { prefix: t, needs_drop_flag: false }
+    }
+    fn may_need_drop_flag(t: Type, needs_drop_flag: bool) -> TypeContext {
+        TypeContext { prefix: t, needs_drop_flag: needs_drop_flag }
+    }
+    pub fn to_string(self) -> String {
+        let TypeContext { prefix, needs_drop_flag } = self;
+        format!("TypeContext {{ prefix: {}, needs_drop_flag: {} }}",
+                prefix.to_string(), needs_drop_flag)
+    }
+}
+
 /// Representations.
 #[derive(Eq, PartialEq, Debug)]
 pub enum Repr<'tcx> {
@@ -125,7 +150,7 @@ pub struct Struct<'tcx> {
     pub align: u32,
     pub sized: bool,
     pub packed: bool,
-    pub fields: Vec<Ty<'tcx>>
+    pub fields: Vec<Ty<'tcx>>,
 }
 
 /// Convenience for `represent_type`.  There should probably be more or
@@ -681,18 +706,30 @@ fn ensure_enum_fits_in_address_space<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
 /// and fill in the actual contents in a second pass to prevent
 /// unbounded recursion; see also the comments in `trans::type_of`.
 pub fn type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, r: &Repr<'tcx>) -> Type {
-    generic_type_of(cx, r, None, false, false)
+    let c = generic_type_of(cx, r, None, false, false, false);
+    assert!(!c.needs_drop_flag);
+    c.prefix
 }
+
+
 // Pass dst=true if the type you are passing is a DST. Yes, we could figure
 // this out, but if you call this on an unsized type without realising it, you
 // are going to get the wrong type (it will not include the unsized parts of it).
 pub fn sizing_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                                 r: &Repr<'tcx>, dst: bool) -> Type {
-    generic_type_of(cx, r, None, true, dst)
+    let c = generic_type_of(cx, r, None, true, dst, false);
+    assert!(!c.needs_drop_flag);
+    c.prefix
+}
+pub fn sizing_type_context_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
+                                        r: &Repr<'tcx>, dst: bool) -> TypeContext {
+    generic_type_of(cx, r, None, true, dst, true)
 }
 pub fn incomplete_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                                     r: &Repr<'tcx>, name: &str) -> Type {
-    generic_type_of(cx, r, Some(name), false, false)
+    let c = generic_type_of(cx, r, Some(name), false, false, false);
+    assert!(!c.needs_drop_flag);
+    c.prefix
 }
 pub fn finish_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                                 r: &Repr<'tcx>, llty: &mut Type) {
@@ -708,20 +745,50 @@ fn generic_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                              r: &Repr<'tcx>,
                              name: Option<&str>,
                              sizing: bool,
-                             dst: bool) -> Type {
+                             dst: bool,
+                             delay_drop_flag: bool) -> TypeContext {
+    debug!("adt::generic_type_of r: {:?} name: {:?} sizing: {} dst: {} delay_drop_flag: {}",
+           r, name, sizing, dst, delay_drop_flag);
     match *r {
-        CEnum(ity, _, _) => ll_inttype(cx, ity),
-        RawNullablePointer { nnty, .. } => type_of::sizing_type_of(cx, nnty),
-        Univariant(ref st, _) | StructWrappedNullablePointer { nonnull: ref st, .. } => {
+        CEnum(ity, _, _) => TypeContext::direct(ll_inttype(cx, ity)),
+        RawNullablePointer { nnty, .. } =>
+            TypeContext::direct(type_of::sizing_type_of(cx, nnty)),
+        StructWrappedNullablePointer { nonnull: ref st, .. } => {
+            match name {
+                None => {
+                    TypeContext::direct(
+                        Type::struct_(cx, &struct_llfields(cx, st, sizing, dst),
+                                      st.packed))
+                }
+                Some(name) => {
+                    assert_eq!(sizing, false);
+                    TypeContext::direct(Type::named_struct(cx, name))
+                }
+            }
+        }
+        Univariant(ref st, dtor_needed) => {
+            let dtor_needed = dtor_needed != 0;
             match name {
                 None => {
-                    Type::struct_(cx, &struct_llfields(cx, st, sizing, dst),
-                                  st.packed)
+                    let mut fields = struct_llfields(cx, st, sizing, dst);
+                    if delay_drop_flag && dtor_needed {
+                        fields.pop();
+                    }
+                    TypeContext::may_need_drop_flag(
+                        Type::struct_(cx, &fields,
+                                      st.packed),
+                        delay_drop_flag && dtor_needed)
+                }
+                Some(name) => {
+                    // Hypothesis: named_struct's can never need a
+                    // drop flag. (... needs validation.)
+                    assert_eq!(sizing, false);
+                    TypeContext::direct(Type::named_struct(cx, name))
                 }
-                Some(name) => { assert_eq!(sizing, false); Type::named_struct(cx, name) }
             }
         }
-        General(ity, ref sts, _) => {
+        General(ity, ref sts, dtor_needed) => {
+            let dtor_needed = dtor_needed != 0;
             // We need a representation that has:
             // * The alignment of the most-aligned field
             // * The size of the largest variant (rounded up to that alignment)
@@ -753,15 +820,25 @@ fn generic_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
             };
             assert_eq!(machine::llalign_of_min(cx, fill_ty), align);
             assert_eq!(align_s % discr_size, 0);
-            let fields = [discr_ty,
-                          Type::array(&discr_ty, align_s / discr_size - 1),
-                          fill_ty];
+            let mut fields: Vec<Type> =
+                [discr_ty,
+                 Type::array(&discr_ty, align_s / discr_size - 1),
+                 fill_ty].iter().cloned().collect();
+            if delay_drop_flag && dtor_needed {
+                fields.pop();
+            }
             match name {
-                None => Type::struct_(cx, &fields[..], false),
+                None => {
+                    TypeContext::may_need_drop_flag(
+                        Type::struct_(cx, &fields[..], false),
+                        delay_drop_flag && dtor_needed)
+                }
                 Some(name) => {
                     let mut llty = Type::named_struct(cx, name);
                     llty.set_struct_body(&fields[..], false);
-                    llty
+                    TypeContext::may_need_drop_flag(
+                        llty,
+                        delay_drop_flag && dtor_needed)
                 }
             }
         }
diff --git a/src/librustc_trans/trans/glue.rs b/src/librustc_trans/trans/glue.rs
index 6019099c058..526c7277a66 100644
--- a/src/librustc_trans/trans/glue.rs
+++ b/src/librustc_trans/trans/glue.rs
@@ -406,8 +406,12 @@ pub fn size_and_align_of_dst<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, t: Ty<'tcx>, in
            t, bcx.val_to_string(info));
     if type_is_sized(bcx.tcx(), t) {
         let sizing_type = sizing_type_of(bcx.ccx(), t);
-        let size = C_uint(bcx.ccx(), llsize_of_alloc(bcx.ccx(), sizing_type));
-        let align = C_uint(bcx.ccx(), align_of(bcx.ccx(), t));
+        let size = llsize_of_alloc(bcx.ccx(), sizing_type);
+        let align = align_of(bcx.ccx(), t);
+        debug!("size_and_align_of_dst t={} info={} size: {} align: {}",
+               t, bcx.val_to_string(info), size, align);
+        let size = C_uint(bcx.ccx(), size);
+        let align = C_uint(bcx.ccx(), align);
         return (size, align);
     }
     match t.sty {
@@ -417,9 +421,14 @@ pub fn size_and_align_of_dst<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, t: Ty<'tcx>, in
             // Don't use type_of::sizing_type_of because that expects t to be sized.
             assert!(!t.is_simd(bcx.tcx()));
             let repr = adt::represent_type(ccx, t);
-            let sizing_type = adt::sizing_type_of(ccx, &*repr, true);
-            let sized_size = C_uint(ccx, llsize_of_alloc(ccx, sizing_type));
-            let sized_align = C_uint(ccx, llalign_of_min(ccx, sizing_type));
+            let sizing_type = adt::sizing_type_context_of(ccx, &*repr, true);
+            debug!("DST {} sizing_type: {}", t, sizing_type.to_string());
+            let sized_size = llsize_of_alloc(ccx, sizing_type.prefix());
+            let sized_align = llalign_of_min(ccx, sizing_type.prefix());
+            debug!("DST {} statically sized prefix size: {} align: {}",
+                   t, sized_size, sized_align);
+            let sized_size = C_uint(ccx, sized_size);
+            let sized_align = C_uint(ccx, sized_align);
 
             // Recurse to get the size of the dynamically sized field (must be
             // the last field).
@@ -428,8 +437,22 @@ pub fn size_and_align_of_dst<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, t: Ty<'tcx>, in
             let field_ty = last_field.mt.ty;
             let (unsized_size, unsized_align) = size_and_align_of_dst(bcx, field_ty, info);
 
+            // #27023 FIXME: We should be adding any necessary padding
+            // to `sized_size` (to accommodate the `unsized_align`
+            // required of the unsized field that follows) before
+            // summing it with `sized_size`.
+
             // Return the sum of sizes and max of aligns.
-            let size = Add(bcx, sized_size, unsized_size, DebugLoc::None);
+            let mut size = Add(bcx, sized_size, unsized_size, DebugLoc::None);
+
+            // Issue #27023: If there is a drop flag, *now* we add 1
+            // to the size.  (We can do this without adding any
+            // padding because drop flags do not have any alignment
+            // constraints.)
+            if sizing_type.needs_drop_flag() {
+                size = Add(bcx, size, C_uint(bcx.ccx(), 1_u64), DebugLoc::None);
+            }
+
             let align = Select(bcx,
                                ICmp(bcx,
                                     llvm::IntULT,
@@ -438,6 +461,11 @@ pub fn size_and_align_of_dst<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, t: Ty<'tcx>, in
                                     DebugLoc::None),
                                sized_align,
                                unsized_align);
+
+            // #27023 FIXME: We should be adding any necessary padding
+            // to `size` (to make it a multiple of `align`) before
+            // returning it.
+
             (size, align)
         }
         ty::TyTrait(..) => {