auto merge of #10652 : jld/rust/enum-unstruct, r=thestinger

This is needed so that the FFI works as expected on platforms that don't
flatten aggregates the way the AMD64 ABI does, especially for `#[repr(C)]`.

This moves more of `type_of` into `trans::adt`, because the type might
or might not be an LLVM struct.

Closes #10308.

3 files changed, 99 insertions, 40 deletions

diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs
index 997bcbca9ce..d75fa9341bf 100644
--- a/src/librustc/middle/trans/adt.rs
+++ b/src/librustc/middle/trans/adt.rs
@@ -366,22 +366,41 @@ pub fn ty_of_inttype(ity: IntType) -> ty::t {
 
 
 /**
- * Returns the fields of a struct for the given representation.
- * All nominal types are LLVM structs, in order to be able to use
- * forward-declared opaque types to prevent circularity in `type_of`.
+ * LLVM-level types are a little complicated.
+ *
+ * C-like enums need to be actual ints, not wrapped in a struct,
+ * because that changes the ABI on some platforms (see issue #10308).
+ *
+ * For nominal types, in some cases, we need to use LLVM named structs
+ * and fill in the actual contents in a second pass to prevent
+ * unbounded recursion; see also the comments in `trans::type_of`.
  */
-pub fn fields_of(cx: &mut CrateContext, r: &Repr) -> ~[Type] {
-    generic_fields_of(cx, r, false)
+pub fn type_of(cx: &mut CrateContext, r: &Repr) -> Type {
+    generic_type_of(cx, r, None, false)
+}
+pub fn sizing_type_of(cx: &mut CrateContext, r: &Repr) -> Type {
+    generic_type_of(cx, r, None, true)
 }
-/// Like `fields_of`, but for `type_of::sizing_type_of` (q.v.).
-pub fn sizing_fields_of(cx: &mut CrateContext, r: &Repr) -> ~[Type] {
-    generic_fields_of(cx, r, true)
+pub fn incomplete_type_of(cx: &mut CrateContext, r: &Repr, name: &str) -> Type {
+    generic_type_of(cx, r, Some(name), false)
+}
+pub fn finish_type_of(cx: &mut CrateContext, r: &Repr, llty: &mut Type) {
+    match *r {
+        CEnum(*) | General(*) => { }
+        Univariant(ref st, _) | NullablePointer{ nonnull: ref st, _ } =>
+            llty.set_struct_body(struct_llfields(cx, st, false), st.packed)
+    }
 }
-fn generic_fields_of(cx: &mut CrateContext, r: &Repr, sizing: bool) -> ~[Type] {
+
+fn generic_type_of(cx: &mut CrateContext, r: &Repr, name: Option<&str>, sizing: bool) -> Type {
     match *r {
-        CEnum(ity, _, _) => ~[ll_inttype(cx, ity)],
-        Univariant(ref st, _dtor) => struct_llfields(cx, st, sizing),
-        NullablePointer{ nonnull: ref st, _ } => struct_llfields(cx, st, sizing),
+        CEnum(ity, _, _) => ll_inttype(cx, ity),
+        Univariant(ref st, _) | NullablePointer{ nonnull: ref st, _ } => {
+            match name {
+                None => Type::struct_(struct_llfields(cx, st, sizing), st.packed),
+                Some(name) => { assert_eq!(sizing, false); Type::named_struct(name) }
+            }
+        }
         General(ity, ref sts) => {
             // We need a representation that has:
             // * The alignment of the most-aligned field
@@ -394,8 +413,7 @@ fn generic_fields_of(cx: &mut CrateContext, r: &Repr, sizing: bool) -> ~[Type] {
             // more of its own type, then use alignment-sized ints to get the rest
             // of the size.
             //
-            // Note: if/when we start exposing SIMD vector types (or f80, on some
-            // platforms that have it), this will need some adjustment.
+            // FIXME #10604: this breaks when vector types are present.
             let size = sts.iter().map(|st| st.size).max().unwrap();
             let most_aligned = sts.iter().max_by(|st| st.align).unwrap();
             let align = most_aligned.align;
@@ -411,9 +429,17 @@ fn generic_fields_of(cx: &mut CrateContext, r: &Repr, sizing: bool) -> ~[Type] {
             assert_eq!(machine::llalign_of_min(cx, pad_ty) as u64, align);
             let align_units = (size + align - 1) / align;
             assert_eq!(align % discr_size, 0);
-            ~[discr_ty,
+            let fields = ~[discr_ty,
               Type::array(&discr_ty, align / discr_size - 1),
-              Type::array(&pad_ty, align_units - 1)]
+              Type::array(&pad_ty, align_units - 1)];
+            match name {
+                None => Type::struct_(fields, false),
+                Some(name) => {
+                    let mut llty = Type::named_struct(name);
+                    llty.set_struct_body(fields, false);
+                    llty
+                }
+            }
         }
     }
 }
@@ -460,7 +486,8 @@ pub fn trans_get_discr(bcx: @mut Block, r: &Repr, scrutinee: ValueRef, cast_to:
             signed = ity.is_signed();
         }
         General(ity, ref cases) => {
-            val = load_discr(bcx, ity, scrutinee, 0, (cases.len() - 1) as Disr);
+            let ptr = GEPi(bcx, scrutinee, [0, 0]);
+            val = load_discr(bcx, ity, ptr, 0, (cases.len() - 1) as Disr);
             signed = ity.is_signed();
         }
         Univariant(*) => {
@@ -487,9 +514,8 @@ fn nullable_bitdiscr(bcx: @mut Block, nonnull: &Struct, nndiscr: Disr, ptrfield:
 }
 
 /// Helper for cases where the discriminant is simply loaded.
-fn load_discr(bcx: @mut Block, ity: IntType, scrutinee: ValueRef, min: Disr, max: Disr)
+fn load_discr(bcx: @mut Block, ity: IntType, ptr: ValueRef, min: Disr, max: Disr)
     -> ValueRef {
-    let ptr = GEPi(bcx, scrutinee, [0, 0]);
     let llty = ll_inttype(bcx.ccx(), ity);
     assert_eq!(val_ty(ptr), llty.ptr_to());
     let bits = machine::llbitsize_of_real(bcx.ccx(), llty);
@@ -546,7 +572,7 @@ pub fn trans_start_init(bcx: @mut Block, r: &Repr, val: ValueRef, discr: Disr) {
         CEnum(ity, min, max) => {
             assert_discr_in_range(ity, min, max, discr);
             Store(bcx, C_integral(ll_inttype(bcx.ccx(), ity), discr as u64, true),
-                  GEPi(bcx, val, [0, 0]))
+                  val)
         }
         General(ity, _) => {
             Store(bcx, C_integral(ll_inttype(bcx.ccx(), ity), discr as u64, true),
diff --git a/src/librustc/middle/trans/type_of.rs b/src/librustc/middle/trans/type_of.rs
index 36af13d34e6..0b51105cdde 100644
--- a/src/librustc/middle/trans/type_of.rs
+++ b/src/librustc/middle/trans/type_of.rs
@@ -147,18 +147,17 @@ pub fn sizing_type_of(cx: &mut CrateContext, t: ty::t) -> Type {
 
         ty::ty_tup(*) | ty::ty_enum(*) => {
             let repr = adt::represent_type(cx, t);
-            Type::struct_(adt::sizing_fields_of(cx, repr), false)
+            adt::sizing_type_of(cx, repr)
         }
 
-        ty::ty_struct(did, _) => {
+        ty::ty_struct(*) => {
             if ty::type_is_simd(cx.tcx, t) {
                 let et = ty::simd_type(cx.tcx, t);
                 let n = ty::simd_size(cx.tcx, t);
                 Type::vector(&type_of(cx, et), n as u64)
             } else {
                 let repr = adt::represent_type(cx, t);
-                let packed = ty::lookup_packed(cx.tcx, did);
-                Type::struct_(adt::sizing_fields_of(cx, repr), packed)
+                adt::sizing_type_of(cx, repr)
             }
         }
 
@@ -217,8 +216,9 @@ pub fn type_of(cx: &mut CrateContext, t: ty::t) -> Type {
         // fill it in *after* placing it into the type cache. This
         // avoids creating more than one copy of the enum when one
         // of the enum's variants refers to the enum itself.
-
-        Type::named_struct(llvm_type_name(cx, an_enum, did, substs.tps))
+        let repr = adt::represent_type(cx, t);
+        let name = llvm_type_name(cx, an_enum, did, substs.tps);
+        adt::incomplete_type_of(cx, repr, name)
       }
       ty::ty_estr(ty::vstore_box) => {
         Type::box(cx, &Type::vec(cx.sess.targ_cfg.arch, &Type::i8())).ptr_to()
@@ -287,7 +287,7 @@ pub fn type_of(cx: &mut CrateContext, t: ty::t) -> Type {
       ty::ty_type => cx.tydesc_type.ptr_to(),
       ty::ty_tup(*) => {
           let repr = adt::represent_type(cx, t);
-          Type::struct_(adt::fields_of(cx, repr), false)
+          adt::type_of(cx, repr)
       }
       ty::ty_opaque_closure_ptr(_) => Type::opaque_box(cx).ptr_to(),
       ty::ty_struct(did, ref substs) => {
@@ -299,7 +299,9 @@ pub fn type_of(cx: &mut CrateContext, t: ty::t) -> Type {
               // Only create the named struct, but don't fill it in. We fill it
               // in *after* placing it into the type cache. This prevents
               // infinite recursion with recursive struct types.
-              Type::named_struct(llvm_type_name(cx, a_struct, did, substs.tps))
+              let repr = adt::represent_type(cx, t);
+              let name = llvm_type_name(cx, a_struct, did, substs.tps);
+              adt::incomplete_type_of(cx, repr, name)
           }
       }
       ty::ty_self(*) => cx.tcx.sess.unimpl("type_of: ty_self"),
@@ -316,19 +318,11 @@ pub fn type_of(cx: &mut CrateContext, t: ty::t) -> Type {
 
     // If this was an enum or struct, fill in the type now.
     match ty::get(t).sty {
-      ty::ty_enum(*) => {
-          let repr = adt::represent_type(cx, t);
-          llty.set_struct_body(adt::fields_of(cx, repr), false);
-      }
-
-      ty::ty_struct(did, _) => {
-        if !ty::type_is_simd(cx.tcx, t) {
-          let repr = adt::represent_type(cx, t);
-          let packed = ty::lookup_packed(cx.tcx, did);
-          llty.set_struct_body(adt::fields_of(cx, repr), packed);
+        ty::ty_enum(*) | ty::ty_struct(*) if !ty::type_is_simd(cx.tcx, t) => {
+            let repr = adt::represent_type(cx, t);
+            adt::finish_type_of(cx, repr, &mut llty);
         }
-      }
-      _ => ()
+        _ => ()
     }
 
     return llty;
diff --git a/src/test/run-pass/enum-clike-ffi-as-int.rs b/src/test/run-pass/enum-clike-ffi-as-int.rs
new file mode 100644
index 00000000000..0c897b959a5
--- /dev/null
+++ b/src/test/run-pass/enum-clike-ffi-as-int.rs
@@ -0,0 +1,39 @@
+// Copyright 2013 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.
+
+/*!
+ * C-like enums have to be represented as LLVM ints, not wrapped in a
+ * struct, because it's important for the FFI that they interoperate
+ * with C integers/enums, and the ABI can treat structs differently.
+ * For example, on i686-linux-gnu, a struct return value is passed by
+ * storing to a hidden out parameter, whereas an integer would be
+ * returned in a register.
+ *
+ * This test just checks that the ABIs for the enum and the plain
+ * integer are compatible, rather than actually calling C code.
+ * The unused parameter to `foo` is to increase the likelihood of
+ * crashing if something goes wrong here.
+ */
+
+#[repr(u32)]
+enum Foo {
+  A = 0,
+  B = 23
+}
+
+#[inline(never)]
+extern "C" fn foo(_x: uint) -> Foo { B }
+
+pub fn main() {
+  unsafe {
+    let f: extern "C" fn(uint) -> u32 = ::std::cast::transmute(foo);
+    assert_eq!(f(0xDEADBEEF), B as u32);
+  }
+}