diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index a6ba742201a3b..644a8c4611c22 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/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) ([], [field]) | ([field], []) => field.ty(cx.tcx, substs), + // If one variant has a ZST and the other has a single field (e.g. Result 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>, 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,6 +874,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { def: ty::AdtDef<'tcx>, variant: &ty::VariantDef, substs: SubstsRef<'tcx>, + allow_unit_type: bool, ) -> FfiResult<'tcx> { use FfiResult::*; @@ -857,7 +882,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // 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,7 +992,14 @@ 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() { @@ -975,17 +1007,24 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { 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`. + // Special-case types like `Option` 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, } diff --git a/tests/ui/abi/nonzero-option-result-ffi.rs b/tests/ui/abi/nonzero-option-result-ffi.rs new file mode 100644 index 0000000000000..66602f9fe6ad0 --- /dev/null +++ b/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()); +} diff --git a/tests/ui/lint/lint-ctypes-enum.rs b/tests/ui/lint/lint-ctypes-enum.rs index 7c20608059398..fd15068cdc9ba 100644 --- a/tests/ui/lint/lint-ctypes-enum.rs +++ b/tests/ui/lint/lint-ctypes-enum.rs @@ -56,37 +56,66 @@ union TransparentUnion { struct Rust(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); - fn nonnull(x: Option>); - fn unique(x: Option>); - fn nonzero_u8(x: Option); - fn nonzero_u16(x: Option); - fn nonzero_u32(x: Option); - fn nonzero_u64(x: Option); - fn nonzero_u128(x: Option); - //~^ ERROR `extern` block uses type `u128` - fn nonzero_usize(x: Option); - fn nonzero_i8(x: Option); - fn nonzero_i16(x: Option); - fn nonzero_i32(x: Option); - fn nonzero_i64(x: Option); - fn nonzero_i128(x: Option); - //~^ ERROR `extern` block uses type `i128` - fn nonzero_isize(x: Option); - fn transparent_struct(x: Option>); - fn transparent_enum(x: Option>); - fn transparent_union(x: Option>); - //~^ ERROR `extern` block uses type - fn repr_rust(x: Option>); //~ 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); + fn nonnull(x: Option>); + fn unique(x: Option>); + fn nonzero_u8(x: Option); + fn nonzero_u16(x: Option); + fn nonzero_u32(x: Option); + fn nonzero_u64(x: Option); + fn nonzero_u128(x: Option); + //~^ ERROR `extern` block uses type `u128` + fn nonzero_usize(x: Option); + fn nonzero_i8(x: Option); + fn nonzero_i16(x: Option); + fn nonzero_i32(x: Option); + fn nonzero_i64(x: Option); + fn nonzero_i128(x: Option); + //~^ 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); + fn left_result_nonzero_u16(x: Result); + fn left_result_nonzero_u32(x: Result); + fn left_result_nonzero_u64(x: Result); + fn left_result_nonzero_u128(x: Result); + //~^ ERROR `extern` block uses type `u128` + fn left_result_nonzero_usize(x: Result); + fn left_result_nonzero_i8(x: Result); + fn left_result_nonzero_i16(x: Result); + fn left_result_nonzero_i32(x: Result); + fn left_result_nonzero_i64(x: Result); + fn left_result_nonzero_i128(x: Result); + //~^ ERROR `extern` block uses type `i128` + fn result_sized(x: Result); + //~^ ERROR `extern` block uses type `Result` + fn left_result_sized(x: Result); + //~^ ERROR `extern` block uses type `Result` + fn nonzero_isize(x: Option); + fn transparent_struct(x: Option>); + fn transparent_enum(x: Option>); + fn transparent_union(x: Option>); + //~^ ERROR `extern` block uses type + fn repr_rust(x: Option>); //~ ERROR `extern` block uses type } pub fn main() {} diff --git a/tests/ui/lint/lint-ctypes-enum.stderr b/tests/ui/lint/lint-ctypes-enum.stderr index 8554e261778e7..93988dbef9cea 100644 --- a/tests/ui/lint/lint-ctypes-enum.stderr +++ b/tests/ui/lint/lint-ctypes-enum.stderr @@ -1,8 +1,8 @@ error: `extern` block uses type `U`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:60:13 + --> $DIR/lint-ctypes-enum.rs:60:14 | -LL | fn uf(x: U); - | ^ not FFI-safe +LL | fn uf(x: U); + | ^ not FFI-safe | = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint @@ -18,10 +18,10 @@ LL | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ error: `extern` block uses type `B`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:61:13 + --> $DIR/lint-ctypes-enum.rs:61:14 | -LL | fn bf(x: B); - | ^ not FFI-safe +LL | fn bf(x: B); + | ^ not FFI-safe | = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint @@ -32,10 +32,10 @@ LL | enum B { | ^^^^^^ error: `extern` block uses type `T`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:62:13 + --> $DIR/lint-ctypes-enum.rs:62:14 | -LL | fn tf(x: T); - | ^ not FFI-safe +LL | fn tf(x: T); + | ^ not FFI-safe | = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint @@ -46,47 +46,88 @@ LL | enum T { | ^^^^^^ error: `extern` block uses type `u128`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:74:23 + --> $DIR/lint-ctypes-enum.rs:74:24 | -LL | fn nonzero_u128(x: Option); - | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe +LL | fn nonzero_u128(x: Option); + | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | = note: 128-bit integers don't currently have a known stable ABI error: `extern` block uses type `i128`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:81:23 + --> $DIR/lint-ctypes-enum.rs:81:24 | -LL | fn nonzero_i128(x: Option); - | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe +LL | fn nonzero_i128(x: Option); + | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | = note: 128-bit integers don't currently have a known stable ABI -error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:86:28 +error: `extern` block uses type `u128`, which is not FFI-safe + --> $DIR/lint-ctypes-enum.rs:87:31 + | +LL | fn result_nonzero_u128(x: Result<(), num::NonZeroU128>); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: `extern` block uses type `i128`, which is not FFI-safe + --> $DIR/lint-ctypes-enum.rs:94:31 | -LL | fn transparent_union(x: Option>); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe +LL | fn result_nonzero_i128(x: Result<(), num::NonZeroI128>); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: `extern` block uses type `u128`, which is not FFI-safe + --> $DIR/lint-ctypes-enum.rs:100:36 + | +LL | fn left_result_nonzero_u128(x: Result); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: `extern` block uses type `i128`, which is not FFI-safe + --> $DIR/lint-ctypes-enum.rs:107:36 + | +LL | fn left_result_nonzero_i128(x: Result); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: `extern` block uses type `Result`, which is not FFI-safe + --> $DIR/lint-ctypes-enum.rs:109:24 + | +LL | fn result_sized(x: Result); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint -error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:88:20 +error: `extern` block uses type `Result`, which is not FFI-safe + --> $DIR/lint-ctypes-enum.rs:111:29 | -LL | fn repr_rust(x: Option>); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe +LL | fn left_result_sized(x: Result); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint -error: `extern` block uses type `Result<(), NonZeroI32>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:89:20 +error: `extern` block uses type `Option>`, which is not FFI-safe + --> $DIR/lint-ctypes-enum.rs:116:29 + | +LL | fn transparent_union(x: Option>); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum + = note: enum has no representation hint + +error: `extern` block uses type `Option>`, which is not FFI-safe + --> $DIR/lint-ctypes-enum.rs:118:21 | -LL | fn no_result(x: Result<(), num::NonZeroI32>); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe +LL | fn repr_rust(x: Option>); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint -error: aborting due to 8 previous errors +error: aborting due to 13 previous errors