Rollup merge of rust-lang#39462 - emilio:improper-ctypes, r=nikomatsakis

lint/ctypes: Don't warn on sized structs with PhantomData.

Fixes rust-lang#34798

src/librustc_lint/types.rs

@@ -339,6 +339,7 @@ struct ImproperCTypesVisitor<'a, 'tcx: 'a> {
 
 enum FfiResult {
     FfiSafe,
+    FfiPhantom,
     FfiUnsafe(&'static str),
     FfiBadStruct(DefId, &'static str),
     FfiBadUnion(DefId, &'static str),
@@ -383,8 +384,11 @@ fn is_repr_nullable_ptr<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
     /// Check if the given type is "ffi-safe" (has a stable, well-defined
     /// representation which can be exported to C code).
-    fn check_type_for_ffi(&self, cache: &mut FxHashSet<Ty<'tcx>>, ty: Ty<'tcx>) -> FfiResult {
+    fn check_type_for_ffi(&self,
+                          cache: &mut FxHashSet<Ty<'tcx>>,
+                          ty: Ty<'tcx>) -> FfiResult {
         use self::FfiResult::*;
+
         let cx = self.cx.tcx;
 
         // Protect against infinite recursion, for example
@@ -397,6 +401,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
 
         match ty.sty {
             ty::TyAdt(def, substs) => {
+                if def.is_phantom_data() {
+                    return FfiPhantom;
+                }
                 match def.adt_kind() {
                     AdtKind::Struct => {
                         if !cx.lookup_repr_hints(def.did).contains(&attr::ReprExtern) {
@@ -405,18 +412,22 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                                               consider adding a #[repr(C)] attribute to the type");
                         }
 
-                        // We can't completely trust repr(C) markings; make sure the
-                        // fields are actually safe.
                         if def.struct_variant().fields.is_empty() {
                             return FfiUnsafe("found zero-size struct in foreign module, consider \
                                               adding a member to this struct");
                         }
 
+                        // We can't completely trust repr(C) markings; make sure the
+                        // fields are actually safe.
+                        let mut all_phantom = true;
                         for field in &def.struct_variant().fields {
                             let field_ty = cx.normalize_associated_type(&field.ty(cx, substs));
                             let r = self.check_type_for_ffi(cache, field_ty);
                             match r {
-                                FfiSafe => {}
+                                FfiSafe => {
+                                    all_phantom = false;
+                                }
+                                FfiPhantom => {}
                                 FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => {
                                     return r;
                                 }
@@ -425,7 +436,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                                 }
                             }
                         }
-                        FfiSafe
+
+                        if all_phantom { FfiPhantom } else { FfiSafe }
                     }
                     AdtKind::Union => {
                         if !cx.lookup_repr_hints(def.did).contains(&attr::ReprExtern) {
@@ -434,11 +446,20 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                                               consider adding a #[repr(C)] attribute to the type");
                         }
 
+                        if def.struct_variant().fields.is_empty() {
+                            return FfiUnsafe("found zero-size union in foreign module, consider \
+                                              adding a member to this union");
+                        }
+
+                        let mut all_phantom = true;
                         for field in &def.struct_variant().fields {
                             let field_ty = cx.normalize_associated_type(&field.ty(cx, substs));
                             let r = self.check_type_for_ffi(cache, field_ty);
                             match r {
-                                FfiSafe => {}
+                                FfiSafe => {
+                                    all_phantom = false;
+                                }
+                                FfiPhantom => {}
                                 FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => {
                                     return r;
                                 }
@@ -447,7 +468,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                                 }
                             }
                         }
-                        FfiSafe
+
+                        if all_phantom { FfiPhantom } else { FfiSafe }
                     }
                     AdtKind::Enum => {
                         if def.variants.is_empty() {
@@ -498,6 +520,10 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                                     FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => {
                                         return r;
                                     }
+                                    FfiPhantom => {
+                                        return FfiBadEnum(def.did,
+                                                          "Found phantom data in enum variant");
+                                    }
                                     FfiUnsafe(s) => {
                                         return FfiBadEnum(def.did, s);
                                     }
@@ -591,6 +617,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
 
         match self.check_type_for_ffi(&mut FxHashSet(), ty) {
             FfiResult::FfiSafe => {}
+            FfiResult::FfiPhantom => {
+                self.cx.span_lint(IMPROPER_CTYPES,
+                                  sp,
+                                  &format!("found zero-sized type composed only \
+                                            of phantom-data in a foreign-function."));
+            }
             FfiResult::FfiUnsafe(s) => {
                 self.cx.span_lint(IMPROPER_CTYPES, sp, s);
             }

src/test/compile-fail/lint-ctypes.rs

@@ -29,6 +29,9 @@ pub type RustBadRet = extern fn() -> Box<u32>;
 pub type CVoidRet = ();
 pub struct Foo;
 
+#[repr(C)]
+pub struct ZeroSizeWithPhantomData(::std::marker::PhantomData<i32>);
+
 extern {
     pub fn ptr_type1(size: *const Foo); //~ ERROR: found struct without
     pub fn ptr_type2(size: *const Foo); //~ ERROR: found struct without
@@ -40,6 +43,9 @@ extern {
     pub fn tuple_type(p: (i32, i32)); //~ ERROR found Rust tuple type
     pub fn tuple_type2(p: I32Pair); //~ ERROR found Rust tuple type
     pub fn zero_size(p: ZeroSize); //~ ERROR found zero-size struct
+    pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); //~ ERROR found zero-sized type
+    pub fn zero_size_phantom_toplevel()
+        -> ::std::marker::PhantomData<bool>; //~ ERROR: found zero-sized type
     pub fn fn_type(p: RustFn); //~ ERROR found function pointer with Rust
     pub fn fn_type2(p: fn()); //~ ERROR found function pointer with Rust
     pub fn fn_contained(p: RustBadRet); //~ ERROR: found struct without

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

@@ -0,0 +1,34 @@
+// Copyright 2017 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.
+
+#![forbid(improper_ctypes)]
+#![allow(dead_code)]
+
+#[repr(C)]
+pub struct Foo {
+    size: u8,
+    __value: ::std::marker::PhantomData<i32>,
+}
+
+#[repr(C)]
+pub struct ZeroSizeWithPhantomData<T>(::std::marker::PhantomData<T>);
+
+#[repr(C)]
+pub struct Bar {
+    size: u8,
+    baz: ZeroSizeWithPhantomData<i32>,
+}
+
+extern "C" {
+    pub fn bar(_: *mut Foo, _: *mut Bar);
+}
+
+fn main() {
+}