Lint non-FFI-safe enums.

Author: jld

src/librustc/middle/lint.rs

@@ -34,6 +34,7 @@
 //! Context itself, span_lint should be used instead of add_lint.
 
 use driver::session;
+use middle::trans::adt; // for `adt::is_ffi_safe`
 use middle::ty;
 use middle::pat_util;
 use metadata::csearch;
@@ -627,6 +628,14 @@ fn check_item_ctypes(cx: &Context, it: &ast::item) {
                                 "found rust type `uint` in foreign module, while \
                                 libc::c_uint or libc::c_ulong should be used");
                     }
+                    ast::DefTy(def_id) => {
+                        if !adt::is_ffi_safe(cx.tcx, def_id) {
+                            cx.span_lint(ctypes, ty.span,
+                                         "found enum type without foreign-function-safe \
+                                          representation annotation in foreign module");
+                            // NOTE this message could be more helpful
+                        }
+                    }
                     _ => ()
                 }
             }

src/librustc/middle/trans/adt.rs

@@ -145,22 +145,8 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr {
             return Univariant(mk_struct(cx, ftys, packed), dtor)
         }
         ty::ty_enum(def_id, ref substs) => {
-            struct Case { discr: Disr, tys: ~[ty::t] };
-            impl Case {
-                fn is_zerolen(&self, cx: &mut CrateContext) -> bool {
-                    mk_struct(cx, self.tys, false).size == 0
-                }
-                fn find_ptr(&self) -> Option<uint> {
-                    self.tys.iter().position(|&ty| mono_data_classify(ty) == MonoNonNull)
-                }
-            }
-
-            let cases = do ty::enum_variants(cx.tcx, def_id).map |vi| {
-                let arg_tys = do vi.args.map |&raw_ty| {
-                    ty::subst(cx.tcx, substs, raw_ty)
-                };
-                Case { discr: vi.disr_val, tys: arg_tys }
-            };
+            let cases = get_cases(cx.tcx, def_id, substs);
+            let hint = ty::lookup_repr_hint(cx.tcx, def_id);
 
             if cases.len() == 0 {
                 // Uninhabitable; represent as unit
@@ -170,7 +156,6 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr {
             if cases.iter().all(|c| c.tys.len() == 0) {
                 // All bodies empty -> intlike
                 let discrs = cases.map(|c| c.discr);
-                let hint = ty::lookup_repr_hint(cx.tcx, def_id);
                 let bounds = IntBounds {
                     ulo: *discrs.iter().min().unwrap(),
                     uhi: *discrs.iter().max().unwrap(),
@@ -232,6 +217,56 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr {
     }
 }
 
+/// Determine, without doing translation, whether an ADT must be FFI-safe.
+/// For use in lint or similar, where being sound but slightly incomplete is acceptable.
+pub fn is_ffi_safe(tcx: ty::ctxt, def_id: ast::DefId) -> bool {
+    match ty::get(ty::lookup_item_type(tcx, def_id).ty).sty {
+        ty::ty_enum(def_id, ref substs) => {
+            let cases = get_cases(tcx, def_id, substs);
+            // Univariant => like struct/tuple.
+            if cases.len() <= 2 {
+                return true;
+            }
+            let hint = ty::lookup_repr_hint(tcx, def_id);
+            // Appropriate representation explicitly selected?
+            if hint.is_ffi_safe() {
+                return true;
+            }
+            // Conservative approximation of nullable pointers, for Option<~T> etc.
+            if cases.len() == 2 && hint == attr::ReprAny &&
+                (cases[0].tys.is_empty() && cases[1].find_ptr().is_some() ||
+                 cases[1].tys.is_empty() && cases[0].find_ptr().is_some()) {
+                return true;
+            }
+            false
+        }
+        // struct, tuple, etc.
+        // (is this right in the present of typedefs?)
+        _ => true
+    }
+}
+
+// NOTE this should probably all be in ty
+struct Case { discr: Disr, tys: ~[ty::t] }
+impl Case {
+    fn is_zerolen(&self, cx: &mut CrateContext) -> bool {
+        mk_struct(cx, self.tys, false).size == 0
+    }
+    fn find_ptr(&self) -> Option<uint> {
+        self.tys.iter().position(|&ty| mono_data_classify(ty) == MonoNonNull)
+    }
+}
+
+fn get_cases(tcx: ty::ctxt, def_id: ast::DefId, substs: &ty::substs) -> ~[Case] {
+    do ty::enum_variants(tcx, def_id).map |vi| {
+        let arg_tys = do vi.args.map |&raw_ty| {
+            ty::subst(tcx, substs, raw_ty)
+        };
+        Case { discr: vi.disr_val, tys: arg_tys }
+    }
+}
+
+
 fn mk_struct(cx: &mut CrateContext, tys: &[ty::t], packed: bool) -> Struct {
     let lltys = tys.map(|&ty| type_of::sizing_type_of(cx, ty));
     let llty_rec = Type::struct_(lltys, packed);

src/libsyntax/attr.rs

@@ -442,6 +442,16 @@ pub enum ReprAttr {
     ReprExtern
 }
 
+impl ReprAttr {
+    pub fn is_ffi_safe(&self) -> bool {
+        match *self {
+            ReprAny => false,
+            ReprInt(_sp, ity) => ity.is_ffi_safe(),
+            ReprExtern => true
+        }
+    }
+}
+
 #[deriving(Eq)]
 pub enum IntType {
     SignedInt(ast::int_ty),
@@ -456,4 +466,13 @@ impl IntType {
             UnsignedInt(*) => false
         }
     }
+    fn is_ffi_safe(self) -> bool {
+        match self {
+            SignedInt(ast::ty_i8) | UnsignedInt(ast::ty_u8) |
+            SignedInt(ast::ty_i16) | UnsignedInt(ast::ty_u16) |
+            SignedInt(ast::ty_i32) | UnsignedInt(ast::ty_u32) |
+            SignedInt(ast::ty_i64) | UnsignedInt(ast::ty_u64) => true,
+            _ => false
+        }
+    }
 }