From 24f90fdd2654e9c5437a684d3a72a4e70826a985 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 19 Jul 2023 09:42:11 +0100 Subject: [PATCH] lint/ctypes: allow `()` within types Consider `()` within types to be FFI-safe, and `()` to be FFI-safe as a return type (incl. when in a transparent newtype). Signed-off-by: David Wood --- compiler/rustc_lint/src/types.rs | 47 ++++++------------- tests/ui/lint/lint-ctypes-113436-1.rs | 28 +++++++++++ tests/ui/lint/lint-ctypes-113436-1.stderr | 35 ++++++++++++++ tests/ui/lint/lint-ctypes-113436.rs | 5 +- tests/ui/lint/lint-ctypes-113436.stderr | 43 ----------------- tests/ui/repr/repr-transparent-issue-87496.rs | 2 +- .../repr/repr-transparent-issue-87496.stderr | 10 ++-- 7 files changed, 87 insertions(+), 83 deletions(-) create mode 100644 tests/ui/lint/lint-ctypes-113436-1.rs create mode 100644 tests/ui/lint/lint-ctypes-113436-1.stderr delete mode 100644 tests/ui/lint/lint-ctypes-113436.stderr diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 7fef88a5391..85bac7588b0 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -943,30 +943,6 @@ pub(crate) fn repr_nullable_ptr<'tcx>( } impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { - // Returns `true` if `ty` is a `()`, or a `repr(transparent)` type whose only non-ZST field - // is a generic substituted for `()` - in either case, the type is FFI-safe when used as a - // return type. - pub fn is_unit_or_equivalent(&self, ty: Ty<'tcx>) -> bool { - if ty.is_unit() { - return true; - } - - if let ty::Adt(def, substs) = ty.kind() && def.repr().transparent() { - return def.variants() - .iter() - .filter_map(|variant| transparent_newtype_field(self.cx.tcx, variant)) - .all(|field| { - let field_ty = field.ty(self.cx.tcx, substs); - !field_ty.has_opaque_types() && { - let field_ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, field_ty); - self.is_unit_or_equivalent(field_ty) - } - }); - } - - false - } - /// Check if the type is array and emit an unsafe type lint. fn check_for_array_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { if let ty::Array(..) = ty.kind() { @@ -1010,14 +986,19 @@ fn check_variant_for_ffi( use FfiResult::*; let transparent_with_all_zst_fields = if def.repr().transparent() { - // Transparent newtypes have at most one non-ZST field which needs to be checked.. if let Some(field) = transparent_newtype_field(self.cx.tcx, variant) { - return self.check_field_type_for_ffi(cache, field, args); - } + // Transparent newtypes have at most one non-ZST field which needs to be checked.. + match self.check_field_type_for_ffi(cache, field, args) { + FfiUnsafe { ty, .. } if ty.is_unit() => (), + r => return r, + } - // ..or have only ZST fields, which is FFI-unsafe (unless those fields are all - // `PhantomData`). - true + false + } else { + // ..or have only ZST fields, which is FFI-unsafe (unless those fields are all + // `PhantomData`). + true + } } else { false }; @@ -1027,6 +1008,8 @@ fn check_variant_for_ffi( for field in &variant.fields { all_phantom &= match self.check_field_type_for_ffi(cache, &field, args) { FfiSafe => false, + // `()` fields are FFI-safe! + FfiUnsafe { ty, .. } if ty.is_unit() => false, FfiPhantom(..) => true, r @ FfiUnsafe { .. } => return r, } @@ -1249,7 +1232,7 @@ fn check_type_for_ffi(&self, cache: &mut FxHashSet>, ty: Ty<'tcx>) -> F } let ret_ty = sig.output(); - if self.is_unit_or_equivalent(ret_ty) { + if ret_ty.is_unit() { return FfiSafe; } @@ -1374,7 +1357,7 @@ fn check_type_for_ffi_and_report_errors( // Don't report FFI errors for unit return types. This check exists here, and not in // the caller (where it would make more sense) so that normalization has definitely // happened. - if is_return_type && self.is_unit_or_equivalent(ty) { + if is_return_type && ty.is_unit() { return; } diff --git a/tests/ui/lint/lint-ctypes-113436-1.rs b/tests/ui/lint/lint-ctypes-113436-1.rs new file mode 100644 index 00000000000..1ca59c6868d --- /dev/null +++ b/tests/ui/lint/lint-ctypes-113436-1.rs @@ -0,0 +1,28 @@ +#![deny(improper_ctypes_definitions)] + +#[repr(C)] +pub struct Foo { + a: u8, + b: (), +} + +extern "C" fn foo(x: Foo) -> Foo { + todo!() +} + +struct NotSafe(u32); + +#[repr(C)] +pub struct Bar { + a: u8, + b: (), + c: NotSafe, +} + +extern "C" fn bar(x: Bar) -> Bar { + //~^ ERROR `extern` fn uses type `NotSafe`, which is not FFI-safe + //~^^ ERROR `extern` fn uses type `NotSafe`, which is not FFI-safe + todo!() +} + +fn main() {} diff --git a/tests/ui/lint/lint-ctypes-113436-1.stderr b/tests/ui/lint/lint-ctypes-113436-1.stderr new file mode 100644 index 00000000000..7b63043f057 --- /dev/null +++ b/tests/ui/lint/lint-ctypes-113436-1.stderr @@ -0,0 +1,35 @@ +error: `extern` fn uses type `NotSafe`, which is not FFI-safe + --> $DIR/lint-ctypes-113436-1.rs:22:22 + | +LL | extern "C" fn bar(x: Bar) -> Bar { + | ^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout +note: the type is defined here + --> $DIR/lint-ctypes-113436-1.rs:13:1 + | +LL | struct NotSafe(u32); + | ^^^^^^^^^^^^^^ +note: the lint level is defined here + --> $DIR/lint-ctypes-113436-1.rs:1:9 + | +LL | #![deny(improper_ctypes_definitions)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `extern` fn uses type `NotSafe`, which is not FFI-safe + --> $DIR/lint-ctypes-113436-1.rs:22:30 + | +LL | extern "C" fn bar(x: Bar) -> Bar { + | ^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout +note: the type is defined here + --> $DIR/lint-ctypes-113436-1.rs:13:1 + | +LL | struct NotSafe(u32); + | ^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/lint/lint-ctypes-113436.rs b/tests/ui/lint/lint-ctypes-113436.rs index d1328af9924..4f733b5bb16 100644 --- a/tests/ui/lint/lint-ctypes-113436.rs +++ b/tests/ui/lint/lint-ctypes-113436.rs @@ -1,3 +1,4 @@ +// check-pass #![deny(improper_ctypes_definitions)] #[repr(C)] @@ -7,20 +8,16 @@ pub struct TransparentWrap(T); pub extern "C" fn f() -> Wrap<()> { - //~^ ERROR `extern` fn uses type `()`, which is not FFI-safe todo!() } const _: extern "C" fn() -> Wrap<()> = f; -//~^ ERROR `extern` fn uses type `()`, which is not FFI-safe pub extern "C" fn ff() -> Wrap> { - //~^ ERROR `extern` fn uses type `()`, which is not FFI-safe todo!() } const _: extern "C" fn() -> Wrap> = ff; -//~^ ERROR `extern` fn uses type `()`, which is not FFI-safe pub extern "C" fn g() -> TransparentWrap<()> { todo!() diff --git a/tests/ui/lint/lint-ctypes-113436.stderr b/tests/ui/lint/lint-ctypes-113436.stderr deleted file mode 100644 index 72f92c1a49c..00000000000 --- a/tests/ui/lint/lint-ctypes-113436.stderr +++ /dev/null @@ -1,43 +0,0 @@ -error: `extern` fn uses type `()`, which is not FFI-safe - --> $DIR/lint-ctypes-113436.rs:9:26 - | -LL | pub extern "C" fn f() -> Wrap<()> { - | ^^^^^^^^ not FFI-safe - | - = help: consider using a struct instead - = note: tuples have unspecified layout -note: the lint level is defined here - --> $DIR/lint-ctypes-113436.rs:1:9 - | -LL | #![deny(improper_ctypes_definitions)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: `extern` fn uses type `()`, which is not FFI-safe - --> $DIR/lint-ctypes-113436.rs:14:10 - | -LL | const _: extern "C" fn() -> Wrap<()> = f; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe - | - = help: consider using a struct instead - = note: tuples have unspecified layout - -error: `extern` fn uses type `()`, which is not FFI-safe - --> $DIR/lint-ctypes-113436.rs:17:27 - | -LL | pub extern "C" fn ff() -> Wrap> { - | ^^^^^^^^^^^^^^ not FFI-safe - | - = help: consider using a struct instead - = note: tuples have unspecified layout - -error: `extern` fn uses type `()`, which is not FFI-safe - --> $DIR/lint-ctypes-113436.rs:22:10 - | -LL | const _: extern "C" fn() -> Wrap> = ff; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe - | - = help: consider using a struct instead - = note: tuples have unspecified layout - -error: aborting due to 4 previous errors - diff --git a/tests/ui/repr/repr-transparent-issue-87496.rs b/tests/ui/repr/repr-transparent-issue-87496.rs index 0ce6fb2c19f..a4dd45c63f5 100644 --- a/tests/ui/repr/repr-transparent-issue-87496.rs +++ b/tests/ui/repr/repr-transparent-issue-87496.rs @@ -6,7 +6,7 @@ struct TransparentCustomZst(()); extern "C" { fn good17(p: TransparentCustomZst); - //~^ WARNING: `extern` block uses type `()`, which is not FFI-safe + //~^ WARNING: `extern` block uses type `TransparentCustomZst`, which is not FFI-safe } fn main() {} diff --git a/tests/ui/repr/repr-transparent-issue-87496.stderr b/tests/ui/repr/repr-transparent-issue-87496.stderr index 03c62f8514e..aee31212b4e 100644 --- a/tests/ui/repr/repr-transparent-issue-87496.stderr +++ b/tests/ui/repr/repr-transparent-issue-87496.stderr @@ -1,11 +1,15 @@ -warning: `extern` block uses type `()`, which is not FFI-safe +warning: `extern` block uses type `TransparentCustomZst`, which is not FFI-safe --> $DIR/repr-transparent-issue-87496.rs:8:18 | LL | fn good17(p: TransparentCustomZst); | ^^^^^^^^^^^^^^^^^^^^ not FFI-safe | - = help: consider using a struct instead - = note: tuples have unspecified layout + = note: this struct contains only zero-sized fields +note: the type is defined here + --> $DIR/repr-transparent-issue-87496.rs:6:1 + | +LL | struct TransparentCustomZst(()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: `#[warn(improper_ctypes)]` on by default warning: 1 warning emitted