rustc: leave space for fields of uninhabited types to allow partial initialization.

Author: eddyb

src/librustc/ty/layout.rs

@@ -325,10 +325,6 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
                         offsets.len(), ty);
                 }
 
-                if field.abi == Abi::Uninhabited {
-                    return Ok(LayoutDetails::uninhabited(fields.len()));
-                }
-
                 if field.is_unsized() {
                     sized = false;
                 }
@@ -451,6 +447,10 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
                 }
             }
 
+            if sized && fields.iter().any(|f| f.abi == Abi::Uninhabited) {
+                abi = Abi::Uninhabited;
+            }
+
             Ok(LayoutDetails {
                 variants: Variants::Single { index: 0 },
                 fields: FieldPlacement::Arbitrary {
@@ -497,7 +497,13 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
 
             // The never type.
             ty::TyNever => {
-                tcx.intern_layout(LayoutDetails::uninhabited(0))
+                tcx.intern_layout(LayoutDetails {
+                    variants: Variants::Single { index: 0 },
+                    fields: FieldPlacement::Union(0),
+                    abi: Abi::Uninhabited,
+                    align: dl.i8_align,
+                    size: Size::from_bytes(0)
+                })
             }
 
             // Potentially-fat pointers.
@@ -711,27 +717,37 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
                     }));
                 }
 
-                let (inh_first, inh_second) = {
-                    let mut inh_variants = (0..variants.len()).filter(|&v| {
-                        variants[v].iter().all(|f| f.abi != Abi::Uninhabited)
+                // A variant is absent if it's uninhabited and only has ZST fields.
+                // Present uninhabited variants only require space for their fields,
+                // but *not* an encoding of the discriminant (e.g. a tag value).
+                // See issue #49298 for more details on the need to leave space
+                // for non-ZST uninhabited data (mostly partial initialization).
+                let absent = |fields: &[TyLayout]| {
+                    let uninhabited = fields.iter().any(|f| f.abi == Abi::Uninhabited);
+                    let is_zst = fields.iter().all(|f| f.is_zst());
+                    uninhabited && is_zst
+                };
+                let (present_first, present_second) = {
+                    let mut present_variants = (0..variants.len()).filter(|&v| {
+                        !absent(&variants[v])
                     });
-                    (inh_variants.next(), inh_variants.next())
+                    (present_variants.next(), present_variants.next())
                 };
-                if inh_first.is_none() {
-                    // Uninhabited because it has no variants, or only uninhabited ones.
-                    return Ok(tcx.intern_layout(LayoutDetails::uninhabited(0)));
+                if present_first.is_none() {
+                    // Uninhabited because it has no variants, or only absent ones.
+                    return tcx.layout_raw(param_env.and(tcx.types.never));
                 }
 
                 let is_struct = !def.is_enum() ||
-                    // Only one variant is inhabited.
-                    (inh_second.is_none() &&
+                    // Only one variant is present.
+                    (present_second.is_none() &&
                     // Representation optimizations are allowed.
                      !def.repr.inhibit_enum_layout_opt());
                 if is_struct {
                     // Struct, or univariant enum equivalent to a struct.
                     // (Typechecking will reject discriminant-sizing attrs.)
 
-                    let v = inh_first.unwrap();
+                    let v = present_first.unwrap();
                     let kind = if def.is_enum() || variants[v].len() == 0 {
                         StructKind::AlwaysSized
                     } else {
@@ -773,7 +789,7 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
 
                     // Find one non-ZST variant.
                     'variants: for (v, fields) in variants.iter().enumerate() {
-                        if fields.iter().any(|f| f.abi == Abi::Uninhabited) {
+                        if absent(fields) {
                             continue 'variants;
                         }
                         for f in fields {
@@ -816,7 +832,7 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
                             let offset = st[i].fields.offset(field_index) + offset;
                             let size = st[i].size;
 
-                            let abi = match st[i].abi {
+                            let mut abi = match st[i].abi {
                                 Abi::Scalar(_) => Abi::Scalar(niche.clone()),
                                 Abi::ScalarPair(ref first, ref second) => {
                                     // We need to use scalar_unit to reset the
@@ -833,6 +849,10 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
                                 _ => Abi::Aggregate { sized: true },
                             };
 
+                            if st.iter().all(|v| v.abi == Abi::Uninhabited) {
+                                abi = Abi::Uninhabited;
+                            }
+
                             return Ok(tcx.intern_layout(LayoutDetails {
                                 variants: Variants::NicheFilling {
                                     dataful_variant: i,
@@ -959,9 +979,6 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
                     let old_ity_size = min_ity.size();
                     let new_ity_size = ity.size();
                     for variant in &mut layout_variants {
-                        if variant.abi == Abi::Uninhabited {
-                            continue;
-                        }
                         match variant.fields {
                             FieldPlacement::Arbitrary { ref mut offsets, .. } => {
                                 for i in offsets {
@@ -1055,6 +1072,11 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
                         }
                     }
                 }
+
+                if layout_variants.iter().all(|v| v.abi == Abi::Uninhabited) {
+                    abi = Abi::Uninhabited;
+                }
+
                 tcx.intern_layout(LayoutDetails {
                     variants: Variants::Tagged {
                         tag,
@@ -1523,9 +1545,14 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx>
                     ty::TyAdt(def, _) => def.variants[variant_index].fields.len(),
                     _ => bug!()
                 };
-                let mut details = LayoutDetails::uninhabited(fields);
-                details.variants = Variants::Single { index: variant_index };
-                cx.tcx().intern_layout(details)
+                let tcx = cx.tcx();
+                tcx.intern_layout(LayoutDetails {
+                    variants: Variants::Single { index: variant_index },
+                    fields: FieldPlacement::Union(fields),
+                    abi: Abi::Uninhabited,
+                    align: tcx.data_layout.i8_align,
+                    size: Size::from_bytes(0)
+                })
             }
 
             Variants::NicheFilling { ref variants, .. } |

src/librustc_target/abi/mod.rs

@@ -761,17 +761,6 @@ impl LayoutDetails {
             align,
         }
     }
-
-    pub fn uninhabited(field_count: usize) -> Self {
-        let align = Align::from_bytes(1, 1).unwrap();
-        LayoutDetails {
-            variants: Variants::Single { index: 0 },
-            fields: FieldPlacement::Union(field_count),
-            abi: Abi::Uninhabited,
-            align,
-            size: Size::from_bytes(0)
-        }
-    }
 }
 
 /// The details of the layout of a type, alongside the type itself.
@@ -826,10 +815,10 @@ impl<'a, Ty> TyLayout<'a, Ty> {
     /// Returns true if the type is a ZST and not unsized.
     pub fn is_zst(&self) -> bool {
         match self.abi {
-            Abi::Uninhabited => true,
             Abi::Scalar(_) |
             Abi::ScalarPair(..) |
             Abi::Vector { .. } => false,
+            Abi::Uninhabited => self.size.bytes() == 0,
             Abi::Aggregate { sized } => sized && self.size.bytes() == 0
         }
     }

src/librustc_trans/mir/operand.rs

@@ -223,12 +223,9 @@ impl<'a, 'tcx> OperandRef<'tcx> {
         let offset = self.layout.fields.offset(i);
 
         let mut val = match (self.val, &self.layout.abi) {
-            // If we're uninhabited, or the field is ZST, it has no data.
-            _ if self.layout.abi == layout::Abi::Uninhabited || field.is_zst() => {
-                return OperandRef {
-                    val: OperandValue::Immediate(C_undef(field.immediate_llvm_type(bx.cx))),
-                    layout: field
-                };
+            // If the field is ZST, it has no data.
+            _ if field.is_zst() => {
+                return OperandRef::new_zst(bx.cx, field);
             }
 
             // Newtype of a scalar, scalar pair or vector.

src/librustc_trans/type_of.rs

@@ -213,10 +213,10 @@ pub trait LayoutLlvmExt<'tcx> {
 impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> {
     fn is_llvm_immediate(&self) -> bool {
         match self.abi {
-            layout::Abi::Uninhabited |
             layout::Abi::Scalar(_) |
             layout::Abi::Vector { .. } => true,
             layout::Abi::ScalarPair(..) => false,
+            layout::Abi::Uninhabited |
             layout::Abi::Aggregate { .. } => self.is_zst()
         }
     }

src/test/run-pass/issue-46845.rs

@@ -24,14 +24,16 @@ union Foo {
 }
 
 // If all the variants are uninhabited, however, the union should be uninhabited.
+// NOTE(#49298) the union being uninhabited shouldn't change its size.
 union Bar {
     _a: (Never, u64),
     _b: (u64, Never)
 }
 
 fn main() {
     assert_eq!(mem::size_of::<Foo>(), 8);
-    assert_eq!(mem::size_of::<Bar>(), 0);
+    // See the note on `Bar`'s definition for why this isn't `0`.
+    assert_eq!(mem::size_of::<Bar>(), 8);
 
     let f = [Foo { a: 42 }, Foo { a: 10 }];
     println!("{}", unsafe { f[0].a });

src/test/run-pass/issue-49298.rs

@@ -0,0 +1,34 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(test)]
+
+extern crate test;
+
+enum Void {}
+
+fn main() {
+    let mut x: (Void, usize);
+    let mut y = 42;
+    x.1 = 13;
+
+    // Make sure `y` stays on the stack.
+    test::black_box(&mut y);
+
+    // Check that the write to `x.1` did not overwrite `y`.
+    // Note that this doesn't fail with optimizations enabled,
+    // because we can't keep `x.1` on the stack, like we can `y`,
+    // as we can't borrow partially initialized variables.
+    assert_eq!(y.to_string(), "42");
+
+    // Check that `(Void, usize)` has space for the `usize` field.
+    assert_eq!(std::mem::size_of::<(Void, usize)>(),
+               std::mem::size_of::<usize>());
+}

src/test/run-pass/issue-50442.rs

@@ -0,0 +1,21 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+enum Void {}
+
+enum Foo {
+    A(i32),
+    B(Void),
+    C(i32)
+}
+
+fn main() {
+    let _foo = Foo::A(0);
+}

src/test/run-pass/type-sizes.rs

@@ -68,9 +68,15 @@ enum EnumSingle5 {
     A = 42 as u8,
 }
 
-enum NicheFilledEnumWithInhabitedVariant {
+enum EnumWithMaybeUninhabitedVariant<T> {
     A(&'static ()),
-    B(&'static (), !),
+    B(&'static (), T),
+    C,
+}
+
+enum NicheFilledEnumWithAbsentVariant {
+    A(&'static ()),
+    B((), !),
     C,
 }
 
@@ -107,5 +113,7 @@ pub fn main() {
     assert_eq!(size_of::<EnumSingle4>(), 1);
     assert_eq!(size_of::<EnumSingle5>(), 1);
 
-    assert_eq!(size_of::<NicheFilledEnumWithInhabitedVariant>(), size_of::<&'static ()>());
+    assert_eq!(size_of::<EnumWithMaybeUninhabitedVariant<!>>(),
+               size_of::<EnumWithMaybeUninhabitedVariant<()>>());
+    assert_eq!(size_of::<NicheFilledEnumWithAbsentVariant>(), size_of::<&'static ()>());
 }