Implement the special repr(C)-non-clike-enum layout

src/librustc/hir/check_attr.rs

@@ -75,7 +75,9 @@ impl<'a> CheckAttrVisitor<'a> {
             }
         };
 
-        let mut conflicting_reprs = 0;
+        let mut int_reprs = 0;
+        let mut is_c = false;
+        let mut is_simd = false;
 
         for word in words {
 
@@ -86,7 +88,7 @@ impl<'a> CheckAttrVisitor<'a> {
 
             let (message, label) = match &*name.as_str() {
                 "C" => {
-                    conflicting_reprs += 1;
+                    is_c = true;
                     if target != Target::Struct &&
                             target != Target::Union &&
                             target != Target::Enum {
@@ -108,7 +110,7 @@ impl<'a> CheckAttrVisitor<'a> {
                     }
                 }
                 "simd" => {
-                    conflicting_reprs += 1;
+                    is_simd = true;
                     if target != Target::Struct {
                         ("attribute should be applied to struct",
                          "a struct")
@@ -128,7 +130,7 @@ impl<'a> CheckAttrVisitor<'a> {
                 "i8" | "u8" | "i16" | "u16" |
                 "i32" | "u32" | "i64" | "u64" |
                 "isize" | "usize" => {
-                    conflicting_reprs += 1;
+                    int_reprs += 1;
                     if target != Target::Enum {
                         ("attribute should be applied to enum",
                          "an enum")
@@ -142,7 +144,11 @@ impl<'a> CheckAttrVisitor<'a> {
                 .span_label(item.span, format!("not {}", label))
                 .emit();
         }
-        if conflicting_reprs > 1 {
+
+        // Warn on repr(u8, u16), repr(C, simd), and c-like-enum-repr(C, u8)
+        if (int_reprs > 1)
+           || (is_simd && is_c)
+           || (int_reprs == 1 && is_c && is_c_like_enum(item)) {
             span_warn!(self.sess, attr.span, E0566,
                        "conflicting representation hints");
         }
@@ -162,3 +168,17 @@ impl<'a> Visitor<'a> for CheckAttrVisitor<'a> {
 pub fn check_crate(sess: &Session, krate: &ast::Crate) {
     visit::walk_crate(&mut CheckAttrVisitor { sess: sess }, krate);
 }
+
+fn is_c_like_enum(item: &ast::Item) -> bool {
+    if let ast::ItemKind::Enum(ref def, _) = item.node {
+        for variant in &def.variants {
+            match variant.node.data {
+                ast::VariantData::Unit(_) => { /* continue */ }
+                _ => { return false; }
+            }
+        }
+        true
+    } else {
+        false
+    }
+}

src/librustc/ty/layout.rs

@@ -942,8 +942,8 @@ impl<'a, 'tcx> LayoutDetails {
             AlwaysSized,
             /// A univariant, the last field of which may be coerced to unsized.
             MaybeUnsized,
-            /// A univariant, but part of an enum.
-            EnumVariant(Integer),
+            /// A univariant, but with a prefix of an arbitrary size & alignment (e.g. enum tag).
+            Prefixed(Size, Align),
         }
         let univariant_uninterned = |fields: &[TyLayout], repr: &ReprOptions, kind| {
             let packed = repr.packed();
@@ -962,14 +962,11 @@ impl<'a, 'tcx> LayoutDetails {
             let mut inverse_memory_index: Vec<u32> = (0..fields.len() as u32).collect();
 
             // Anything with repr(C) or repr(packed) doesn't optimize.
-            let optimize = match kind {
-                StructKind::AlwaysSized |
-                StructKind::MaybeUnsized |
-                StructKind::EnumVariant(I8) => {
-                    (repr.flags & ReprFlags::IS_UNOPTIMISABLE).is_empty()
-                }
-                StructKind::EnumVariant(_) => false
-            };
+            let mut optimize = (repr.flags & ReprFlags::IS_UNOPTIMISABLE).is_empty();
+            if let StructKind::Prefixed(_, align) = kind {
+                optimize &= align.abi() == 1;
+            }
+
             if optimize {
                 let end = if let StructKind::MaybeUnsized = kind {
                     fields.len() - 1
@@ -987,7 +984,7 @@ impl<'a, 'tcx> LayoutDetails {
                             (!f.is_zst(), cmp::Reverse(f.align.abi()))
                         })
                     }
-                    StructKind::EnumVariant(_) => {
+                    StructKind::Prefixed(..) => {
                         optimizing.sort_by_key(|&x| fields[x as usize].align.abi());
                     }
                 }
@@ -1001,12 +998,11 @@ impl<'a, 'tcx> LayoutDetails {
 
             let mut offset = Size::from_bytes(0);
 
-            if let StructKind::EnumVariant(discr) = kind {
-                offset = discr.size();
+            if let StructKind::Prefixed(prefix_size, prefix_align) = kind {
                 if !packed {
-                    let discr_align = discr.align(dl);
-                    align = align.max(discr_align);
+                    align = align.max(prefix_align);
                 }
+                offset = prefix_size.abi_align(prefix_align);
             }
 
             for &i in &inverse_memory_index {
@@ -1558,10 +1554,24 @@ impl<'a, 'tcx> LayoutDetails {
                 let mut start_align = Align::from_bytes(256, 256).unwrap();
                 assert_eq!(Integer::for_abi_align(dl, start_align), None);
 
+                // repr(C) on an enum tells us to make a (tag, union) layout,
+                // so we need to grow the prefix alignment to be at least
+                // the alignment of the union. (This value is used both for
+                // determining the alignment of the overall enum, and the
+                // determining the alignment of the payload after the tag.)
+                let mut prefix_align = min_ity.align(dl);
+                if def.repr.c() {
+                    for fields in &variants {
+                        for field in fields {
+                            prefix_align = prefix_align.max(field.align);
+                        }
+                    }
+                }
+
                 // Create the set of structs that represent each variant.
                 let mut variants = variants.into_iter().enumerate().map(|(i, field_layouts)| {
                     let mut st = univariant_uninterned(&field_layouts,
-                        &def.repr, StructKind::EnumVariant(min_ity))?;
+                        &def.repr, StructKind::Prefixed(min_ity.size(), prefix_align))?;
                     st.variants = Variants::Single { index: i };
                     // Find the first field we can't move later
                     // to make room for a larger discriminant.

src/test/run-pass/enum-non-c-like-repr-c-and-int.rs

@@ -0,0 +1,177 @@
+// Copyright 2012 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.
+
+// This test deserializes an enum in-place by transmuting to a union that
+// should have the same layout, and manipulating the tag and payloads
+// independently. This verifies that `repr(some_int)` has a stable representation,
+// and that we don't miscompile these kinds of manipulations.
+
+use std::time::Duration;
+use std::mem;
+
+#[repr(C, u8)]
+#[derive(Copy, Clone, Eq, PartialEq, Debug)]
+enum MyEnum {
+    A(u32),                 // Single primitive value
+    B { x: u8, y: i16 },    // Composite, and the offset of `y` depends on tag being internal
+    C,                      // Empty
+    D(Option<u32>),         // Contains an enum
+    E(Duration),            // Contains a struct
+}
+
+#[repr(C)]
+struct MyEnumRepr {
+    tag: MyEnumTag,
+    payload: MyEnumPayload,
+}
+
+#[repr(C)]
+#[allow(non_snake_case)]
+union MyEnumPayload {
+    A: MyEnumVariantA,
+    B: MyEnumVariantB,
+    D: MyEnumVariantD,
+    E: MyEnumVariantE,
+}
+
+#[repr(u8)] #[derive(Copy, Clone)] enum MyEnumTag { A, B, C, D, E }
+#[repr(C)] #[derive(Copy, Clone)] struct MyEnumVariantA(u32);
+#[repr(C)] #[derive(Copy, Clone)] struct MyEnumVariantB {x: u8, y: i16 }
+#[repr(C)] #[derive(Copy, Clone)] struct MyEnumVariantD(Option<u32>);
+#[repr(C)] #[derive(Copy, Clone)] struct MyEnumVariantE(Duration);
+
+fn main() {
+    let result: Vec<Result<MyEnum, ()>> = vec![
+        Ok(MyEnum::A(17)),
+        Ok(MyEnum::B { x: 206, y: 1145 }),
+        Ok(MyEnum::C),
+        Err(()),
+        Ok(MyEnum::D(Some(407))),
+        Ok(MyEnum::D(None)),
+        Ok(MyEnum::E(Duration::from_secs(100))),
+        Err(()),
+    ];
+
+    // Binary serialized version of the above (little-endian)
+    let input: Vec<u8> = vec![
+        0,  17, 0, 0, 0,
+        1,  206,  121, 4,
+        2,
+        8,  /* invalid tag value */
+        3,  0,  151, 1, 0, 0,
+        3,  1,
+        4,  100, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,
+        0,  /* incomplete value */
+    ];
+
+    let mut output = vec![];
+    let mut buf = &input[..];
+
+    unsafe {
+        // This should be safe, because we don't match on it unless it's fully formed,
+        // and it doesn't have a destructor.
+        let mut dest: MyEnum = mem::uninitialized();
+        while buf.len() > 0 {
+            match parse_my_enum(&mut dest, &mut buf) {
+                Ok(()) => output.push(Ok(dest)),
+                Err(()) => output.push(Err(())),
+            }
+        }
+    }
+
+    assert_eq!(output, result);
+}
+
+fn parse_my_enum<'a>(dest: &'a mut MyEnum, buf: &mut &[u8]) -> Result<(), ()> {
+    unsafe {
+        // Should be correct to do this transmute.
+        let dest: &'a mut MyEnumRepr = mem::transmute(dest);
+        let tag = read_u8(buf)?;
+
+        dest.tag = match tag {
+            0 => MyEnumTag::A,
+            1 => MyEnumTag::B,
+            2 => MyEnumTag::C,
+            3 => MyEnumTag::D,
+            4 => MyEnumTag::E,
+            _ => return Err(()),
+        };
+
+        match dest.tag {
+            MyEnumTag::A => {
+                dest.payload.A.0 = read_u32_le(buf)?;
+            }
+            MyEnumTag::B => {
+                dest.payload.B.x = read_u8(buf)?;
+                dest.payload.B.y = read_u16_le(buf)? as i16;
+            }
+            MyEnumTag::C => {
+                /* do nothing */
+            }
+            MyEnumTag::D => {
+                let is_some = read_u8(buf)? == 0;
+                if is_some {
+                    dest.payload.D.0 = Some(read_u32_le(buf)?);
+                } else {
+                    dest.payload.D.0 = None;
+                }
+            }
+            MyEnumTag::E => {
+                let secs = read_u64_le(buf)?;
+                let nanos = read_u32_le(buf)?;
+                dest.payload.E.0 = Duration::new(secs, nanos);
+            }
+        }
+        Ok(())
+    }
+}
+
+
+
+// reader helpers
+
+fn read_u64_le(buf: &mut &[u8]) -> Result<u64, ()> {
+    if buf.len() < 8 { return Err(()) }
+    let val = (buf[0] as u64) << 0
+            | (buf[1] as u64) << 8
+            | (buf[2] as u64) << 16
+            | (buf[3] as u64) << 24
+            | (buf[4] as u64) << 32
+            | (buf[5] as u64) << 40
+            | (buf[6] as u64) << 48
+            | (buf[7] as u64) << 56;
+    *buf = &buf[8..];
+    Ok(val)
+}
+
+fn read_u32_le(buf: &mut &[u8]) -> Result<u32, ()> {
+    if buf.len() < 4 { return Err(()) }
+    let val = (buf[0] as u32) << 0
+            | (buf[1] as u32) << 8
+            | (buf[2] as u32) << 16
+            | (buf[3] as u32) << 24;
+    *buf = &buf[4..];
+    Ok(val)
+}
+
+fn read_u16_le(buf: &mut &[u8]) -> Result<u16, ()> {
+    if buf.len() < 2 { return Err(()) }
+    let val = (buf[0] as u16) << 0
+            | (buf[1] as u16) << 8;
+    *buf = &buf[2..];
+    Ok(val)
+}
+
+fn read_u8(buf: &mut &[u8]) -> Result<u8, ()> {
+    if buf.len() < 1 { return Err(()) }
+    let val = buf[0];
+    *buf = &buf[1..];
+    Ok(val)
+}