Guarantee result nonzero for FFI

compiler/rustc_lint/src/types.rs

@@ -15,11 +15,14 @@ use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::DiagnosticMessage;
 use rustc_hir as hir;
 use rustc_hir::{is_range_literal, Expr, ExprKind, Node};
-use rustc_middle::ty::layout::{IntegerExt, LayoutOf, SizeSkeleton};
-use rustc_middle::ty::subst::SubstsRef;
 use rustc_middle::ty::{
     self, AdtKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt,
 };
+use rustc_middle::ty::{
+    layout::{IntegerExt, LayoutOf, SizeSkeleton},
+    VariantDef,
+};
+use rustc_middle::ty::{subst::SubstsRef, FieldDef};
 use rustc_span::def_id::LocalDefId;
 use rustc_span::source_map;
 use rustc_span::symbol::sym;
@@ -770,8 +773,26 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
     debug!("is_repr_nullable_ptr(cx, ty = {:?})", ty);
     if let ty::Adt(ty_def, substs) = ty.kind() {
         let field_ty = match &ty_def.variants().raw[..] {
-            [var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
+            [variant1, variant2] => match (&variant1.fields.raw[..], &variant2.fields.raw[..]) {
+                // If one variant is empty and the other is a single field (e.g. Option<T>)
                 ([], [field]) | ([field], []) => field.ty(cx.tcx, substs),
+                // If one variant has a ZST and the other has a single field (e.g. Result<T, ()> or Result<(), T>)
+                ([field1], [field2]) => {
+                    let is_zst = |field: &FieldDef, variant: &VariantDef| {
+                        let param_env = cx.tcx.param_env(variant.def_id);
+                        let field_ty = field.ty(cx.tcx, substs);
+                        cx.tcx
+                            .layout_of(param_env.and(field_ty))
+                            .map_or(false, |layout| layout.is_zst())
+                    };
+                    if is_zst(field1, variant1) {
+                        field2.ty(cx.tcx, substs)
+                    } else if is_zst(field2, variant2) {
+                        field1.ty(cx.tcx, substs)
+                    } else {
+                        return None;
+                    }
+                }
                 _ => return None,
             },
             _ => return None,
@@ -832,9 +853,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         cache: &mut FxHashSet<Ty<'tcx>>,
         field: &ty::FieldDef,
         substs: SubstsRef<'tcx>,
+        allow_unit_type: bool,
     ) -> FfiResult<'tcx> {
         let field_ty = field.ty(self.cx.tcx, substs);
-        if field_ty.has_opaque_types() {
+        if field_ty.is_unit() && allow_unit_type {
+            FfiResult::FfiSafe
+        } else if field_ty.has_opaque_types() {
             self.check_type_for_ffi(cache, field_ty)
         } else {
             let field_ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, field_ty);
@@ -850,14 +874,15 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         def: ty::AdtDef<'tcx>,
         variant: &ty::VariantDef,
         substs: SubstsRef<'tcx>,
+        allow_unit_type: bool,
     ) -> FfiResult<'tcx> {
         use FfiResult::*;
 
         let transparent_safety = def.repr().transparent().then(|| {
             // Can assume that at most one field is not a ZST, so only check
             // that field's type for FFI-safety.
             if let Some(field) = transparent_newtype_field(self.cx.tcx, variant) {
-                return self.check_field_type_for_ffi(cache, field, substs);
+                return self.check_field_type_for_ffi(cache, field, substs, allow_unit_type);
             } else {
                 // All fields are ZSTs; this means that the type should behave
                 // like (), which is FFI-unsafe... except if all fields are PhantomData,
@@ -869,7 +894,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         // actually safe.
         let mut all_phantom = !variant.fields.is_empty();
         for field in &variant.fields {
-            match self.check_field_type_for_ffi(cache, &field, substs) {
+            match self.check_field_type_for_ffi(cache, &field, substs, allow_unit_type) {
                 FfiSafe => {
                     all_phantom = false;
                 }
@@ -967,25 +992,39 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                             };
                         }
 
-                        self.check_variant_for_ffi(cache, ty, def, def.non_enum_variant(), substs)
+                        self.check_variant_for_ffi(
+                            cache,
+                            ty,
+                            def,
+                            def.non_enum_variant(),
+                            substs,
+                            false,
+                        )
                     }
                     AdtKind::Enum => {
                         if def.variants().is_empty() {
                             // Empty enums are okay... although sort of useless.
                             return FfiSafe;
                         }
 
+                        // We need to keep track if this enum is a represented as a nullable
+                        // pointer (or nonzero integer). This is because when we have an enum
+                        // such as Result<(), NonZeroU32>, we allow this to compile since the FFI
+                        // representation of this is as a u32
+                        let mut is_repr_nullable_ptr = false;
                         // Check for a repr() attribute to specify the size of the
                         // discriminant.
                         if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none()
                         {
-                            // Special-case types like `Option<extern fn()>`.
+                            // Special-case types like `Option<extern fn()>` and `Result<(), NonZeroU32>`.
                             if repr_nullable_ptr(self.cx, ty, self.mode).is_none() {
                                 return FfiUnsafe {
                                     ty,
                                     reason: fluent::lint_improper_ctypes_enum_repr_reason,
                                     help: Some(fluent::lint_improper_ctypes_enum_repr_help),
                                 };
+                            } else {
+                                is_repr_nullable_ptr = true;
                             }
                         }
 
@@ -1008,7 +1047,14 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                                 };
                             }
 
-                            match self.check_variant_for_ffi(cache, ty, def, variant, substs) {
+                            match self.check_variant_for_ffi(
+                                cache,
+                                ty,
+                                def,
+                                variant,
+                                substs,
+                                is_repr_nullable_ptr,
+                            ) {
                                 FfiSafe => (),
                                 r => return r,
                             }

tests/ui/abi/nonzero-option-result-ffi.rs

@@ -0,0 +1,125 @@
+// run-pass
+
+use std::mem;
+use std::num::*;
+
+macro_rules! define_with_type {
+    ($namespace: ident, $ty: ty, $abi_ty: ty) => {
+        mod $namespace {
+            #[allow(unused)]
+            use super::*;
+
+            #[inline(never)]
+            pub extern "C" fn option(is_some: bool, value: $ty) -> Option<$ty> {
+                if is_some {
+                    Some(value)
+                } else {
+                    None
+                }
+            }
+
+            #[inline(never)]
+            pub extern "C" fn result_pos(is_ok: bool, value: $ty) -> Result<$ty, ()> {
+                if is_ok {
+                    Ok(value)
+                } else {
+                    Err(())
+                }
+            }
+
+            #[inline(never)]
+            pub extern "C" fn result_neg(is_ok: bool, value: $ty) -> Result<(), $ty> {
+                if is_ok {
+                    Ok(())
+                } else {
+                    Err(value)
+                }
+            }
+
+            #[inline(never)]
+            pub extern "C" fn option_param(value: Option<$ty>) -> $abi_ty {
+                unsafe { mem::transmute(value) }
+            }
+
+            #[inline(never)]
+            pub extern "C" fn result_param_pos(value: Result<$ty, ()>) -> $abi_ty {
+                unsafe { mem::transmute(value) }
+            }
+
+            #[inline(never)]
+            pub extern "C" fn result_param_neg(value: Result<(), $ty>) -> $abi_ty {
+                unsafe { mem::transmute(value) }
+            }
+        }
+    };
+}
+
+define_with_type!(nonzero_i8, NonZeroI8, i8);
+define_with_type!(nonzero_i16, NonZeroI16, i16);
+define_with_type!(nonzero_i32, NonZeroI32, i32);
+define_with_type!(nonzero_i64, NonZeroI64, i64);
+define_with_type!(nonzero_u8, NonZeroU8, u8);
+define_with_type!(nonzero_u16, NonZeroU16, u16);
+define_with_type!(nonzero_u32, NonZeroU32, u32);
+define_with_type!(nonzero_u64, NonZeroU64, u64);
+define_with_type!(nonzero_usize, NonZeroUsize, usize);
+define_with_type!(nonzero_ref, &'static i32, *const i32);
+
+pub fn main() {
+    macro_rules! test_with_type {
+        (
+            $namespace: ident,
+            $ty: ty,
+            $abi_ty: ty,
+            $in_value: expr,
+            $out_value: expr,
+            $null_value:expr
+        ) => {
+            let in_value: $ty = $in_value;
+            let out_value: $abi_ty = $out_value;
+            let null_value: $abi_ty = $null_value;
+            unsafe {
+                let f: extern "C" fn(bool, $ty) -> $abi_ty =
+                    mem::transmute($namespace::option as extern "C" fn(bool, $ty) -> Option<$ty>);
+                assert_eq!(f(true, in_value), out_value);
+                assert_eq!(f(false, in_value), null_value);
+            }
+
+            unsafe {
+                let f: extern "C" fn(bool, $ty) -> $abi_ty = mem::transmute(
+                    $namespace::result_pos as extern "C" fn(bool, $ty) -> Result<$ty, ()>,
+                );
+                assert_eq!(f(true, in_value), out_value);
+                assert_eq!(f(false, in_value), null_value);
+            }
+
+            unsafe {
+                let f: extern "C" fn(bool, $ty) -> $abi_ty = mem::transmute(
+                    $namespace::result_neg as extern "C" fn(bool, $ty) -> Result<(), $ty>,
+                );
+                assert_eq!(f(false, in_value), out_value);
+                assert_eq!(f(true, in_value), null_value);
+            }
+
+            assert_eq!($namespace::option_param(Some(in_value)), out_value);
+            assert_eq!($namespace::option_param(None), null_value);
+
+            assert_eq!($namespace::result_param_pos(Ok(in_value)), out_value);
+            assert_eq!($namespace::result_param_pos(Err(())), null_value);
+
+            assert_eq!($namespace::result_param_neg(Err(in_value)), out_value);
+            assert_eq!($namespace::result_param_neg(Ok(())), null_value);
+        };
+    }
+    test_with_type!(nonzero_i8, NonZeroI8, i8, NonZeroI8::new(123).unwrap(), 123, 0);
+    test_with_type!(nonzero_i16, NonZeroI16, i16, NonZeroI16::new(1234).unwrap(), 1234, 0);
+    test_with_type!(nonzero_i32, NonZeroI32, i32, NonZeroI32::new(1234).unwrap(), 1234, 0);
+    test_with_type!(nonzero_i64, NonZeroI64, i64, NonZeroI64::new(1234).unwrap(), 1234, 0);
+    test_with_type!(nonzero_u8, NonZeroU8, u8, NonZeroU8::new(123).unwrap(), 123, 0);
+    test_with_type!(nonzero_u16, NonZeroU16, u16, NonZeroU16::new(1234).unwrap(), 1234, 0);
+    test_with_type!(nonzero_u32, NonZeroU32, u32, NonZeroU32::new(1234).unwrap(), 1234, 0);
+    test_with_type!(nonzero_u64, NonZeroU64, u64, NonZeroU64::new(1234).unwrap(), 1234, 0);
+    test_with_type!(nonzero_usize, NonZeroUsize, usize, NonZeroUsize::new(1234).unwrap(), 1234, 0);
+    static FOO: i32 = 0xDEADBEE;
+    test_with_type!(nonzero_ref, &'static i32, *const i32, &FOO, &FOO, std::ptr::null());
+}

tests/ui/lint/lint-ctypes-enum.rs

@@ -56,37 +56,66 @@ union TransparentUnion<T: Copy> {
 struct Rust<T>(T);
 
 extern "C" {
-   fn zf(x: Z);
-   fn uf(x: U); //~ ERROR `extern` block uses type `U`
-   fn bf(x: B); //~ ERROR `extern` block uses type `B`
-   fn tf(x: T); //~ ERROR `extern` block uses type `T`
-   fn repr_c(x: ReprC);
-   fn repr_u8(x: U8);
-   fn repr_isize(x: Isize);
-   fn option_ref(x: Option<&'static u8>);
-   fn option_fn(x: Option<extern "C" fn()>);
-   fn nonnull(x: Option<std::ptr::NonNull<u8>>);
-   fn unique(x: Option<std::ptr::Unique<u8>>);
-   fn nonzero_u8(x: Option<num::NonZeroU8>);
-   fn nonzero_u16(x: Option<num::NonZeroU16>);
-   fn nonzero_u32(x: Option<num::NonZeroU32>);
-   fn nonzero_u64(x: Option<num::NonZeroU64>);
-   fn nonzero_u128(x: Option<num::NonZeroU128>);
-   //~^ ERROR `extern` block uses type `u128`
-   fn nonzero_usize(x: Option<num::NonZeroUsize>);
-   fn nonzero_i8(x: Option<num::NonZeroI8>);
-   fn nonzero_i16(x: Option<num::NonZeroI16>);
-   fn nonzero_i32(x: Option<num::NonZeroI32>);
-   fn nonzero_i64(x: Option<num::NonZeroI64>);
-   fn nonzero_i128(x: Option<num::NonZeroI128>);
-   //~^ ERROR `extern` block uses type `i128`
-   fn nonzero_isize(x: Option<num::NonZeroIsize>);
-   fn transparent_struct(x: Option<TransparentStruct<num::NonZeroU8>>);
-   fn transparent_enum(x: Option<TransparentEnum<num::NonZeroU8>>);
-   fn transparent_union(x: Option<TransparentUnion<num::NonZeroU8>>);
-   //~^ ERROR `extern` block uses type
-   fn repr_rust(x: Option<Rust<num::NonZeroU8>>); //~ ERROR `extern` block uses type
-   fn no_result(x: Result<(), num::NonZeroI32>); //~ ERROR `extern` block uses type
+    fn zf(x: Z);
+    fn uf(x: U); //~ ERROR `extern` block uses type `U`
+    fn bf(x: B); //~ ERROR `extern` block uses type `B`
+    fn tf(x: T); //~ ERROR `extern` block uses type `T`
+    fn repr_c(x: ReprC);
+    fn repr_u8(x: U8);
+    fn repr_isize(x: Isize);
+    fn option_ref(x: Option<&'static u8>);
+    fn option_fn(x: Option<extern "C" fn()>);
+    fn nonnull(x: Option<std::ptr::NonNull<u8>>);
+    fn unique(x: Option<std::ptr::Unique<u8>>);
+    fn nonzero_u8(x: Option<num::NonZeroU8>);
+    fn nonzero_u16(x: Option<num::NonZeroU16>);
+    fn nonzero_u32(x: Option<num::NonZeroU32>);
+    fn nonzero_u64(x: Option<num::NonZeroU64>);
+    fn nonzero_u128(x: Option<num::NonZeroU128>);
+    //~^ ERROR `extern` block uses type `u128`
+    fn nonzero_usize(x: Option<num::NonZeroUsize>);
+    fn nonzero_i8(x: Option<num::NonZeroI8>);
+    fn nonzero_i16(x: Option<num::NonZeroI16>);
+    fn nonzero_i32(x: Option<num::NonZeroI32>);
+    fn nonzero_i64(x: Option<num::NonZeroI64>);
+    fn nonzero_i128(x: Option<num::NonZeroI128>);
+    //~^ ERROR `extern` block uses type `i128`
+    fn result_nonzero_u8(x: Result<(), num::NonZeroU8>);
+    fn result_nonzero_u16(x: Result<(), num::NonZeroU16>);
+    fn result_nonzero_u32(x: Result<(), num::NonZeroU32>);
+    fn result_nonzero_u64(x: Result<(), num::NonZeroU64>);
+    fn result_nonzero_u128(x: Result<(), num::NonZeroU128>);
+    //~^ ERROR `extern` block uses type `u128`
+    fn result_nonzero_usize(x: Result<(), num::NonZeroUsize>);
+    fn result_nonzero_i8(x: Result<(), num::NonZeroI8>);
+    fn result_nonzero_i16(x: Result<(), num::NonZeroI16>);
+    fn result_nonzero_i32(x: Result<(), num::NonZeroI32>);
+    fn result_nonzero_i64(x: Result<(), num::NonZeroI64>);
+    fn result_nonzero_i128(x: Result<(), num::NonZeroI128>);
+    //~^ ERROR `extern` block uses type `i128`
+    fn left_result_nonzero_u8(x: Result<num::NonZeroU8, ()>);
+    fn left_result_nonzero_u16(x: Result<num::NonZeroU16, ()>);
+    fn left_result_nonzero_u32(x: Result<num::NonZeroU32, ()>);
+    fn left_result_nonzero_u64(x: Result<num::NonZeroU64, ()>);
+    fn left_result_nonzero_u128(x: Result<num::NonZeroU128, ()>);
+    //~^ ERROR `extern` block uses type `u128`
+    fn left_result_nonzero_usize(x: Result<num::NonZeroUsize, ()>);
+    fn left_result_nonzero_i8(x: Result<num::NonZeroI8, ()>);
+    fn left_result_nonzero_i16(x: Result<num::NonZeroI16, ()>);
+    fn left_result_nonzero_i32(x: Result<num::NonZeroI32, ()>);
+    fn left_result_nonzero_i64(x: Result<num::NonZeroI64, ()>);
+    fn left_result_nonzero_i128(x: Result<num::NonZeroI128, ()>);
+    //~^ ERROR `extern` block uses type `i128`
+    fn result_sized(x: Result<i32, num::NonZeroI128>);
+    //~^ ERROR `extern` block uses type `Result<i32, NonZeroI128>`
+    fn left_result_sized(x: Result<num::NonZeroI128, i32>);
+    //~^ ERROR `extern` block uses type `Result<NonZeroI128, i32>`
+    fn nonzero_isize(x: Option<num::NonZeroIsize>);
+    fn transparent_struct(x: Option<TransparentStruct<num::NonZeroU8>>);
+    fn transparent_enum(x: Option<TransparentEnum<num::NonZeroU8>>);
+    fn transparent_union(x: Option<TransparentUnion<num::NonZeroU8>>);
+    //~^ ERROR `extern` block uses type
+    fn repr_rust(x: Option<Rust<num::NonZeroU8>>); //~ ERROR `extern` block uses type
 }
 
 pub fn main() {}