From e9cf280ef2ea4550d2305484873a63dfd133abb7 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 16 Oct 2023 23:56:22 -0700 Subject: [PATCH] warn less about non-exhaustive in ffi Bindgen allows generating `#[non_exhaustive] #[repr(u32)]` enums. This results in nonintuitive nonlocal `improper_ctypes` warnings, even when the types are otherwise perfectly valid in C. Adjust for actual tooling expectations by avoiding warning on simple enums with only unit variants. --- compiler/rustc_lint/src/types.rs | 27 ++++++++++++------- .../improper_ctypes/auxiliary/types.rs | 11 ++++++++ .../improper_ctypes/extern_crate_improper.rs | 10 ++++++- .../extern_crate_improper.stderr | 10 +++---- 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 900c496e033..eef1f0d133a 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -3,6 +3,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_errors::DiagMessage; +use rustc_hir::def::CtorKind; use rustc_hir::{is_range_literal, Expr, ExprKind, Node}; use rustc_middle::bug; use rustc_middle::ty::layout::{IntegerExt, LayoutOf, SizeSkeleton}; @@ -1386,15 +1387,6 @@ fn check_type_for_ffi( // Empty enums are okay... although sort of useless. return FfiSafe; } - - if def.is_variant_list_non_exhaustive() && !def.did().is_local() { - return FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_non_exhaustive, - help: None, - }; - } - // Check for a repr() attribute to specify the size of the // discriminant. if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none() @@ -1413,8 +1405,25 @@ fn check_type_for_ffi( }; } + // non_exhaustive suggests it is possible that someone might break ABI + // see: https://github.com/rust-lang/rust/issues/44109#issuecomment-537583344 + // so warn on complex enums being used outside their crate + let nonexhaustive_nonlocal_ffi = + def.is_variant_list_non_exhaustive() && !def.did().is_local(); + // Check the contained variants. for variant in def.variants() { + // but only warn about really_tagged_union reprs, + // exempt enums with unit ctors like C's (like rust-bindgen) + if nonexhaustive_nonlocal_ffi + && !matches!(variant.ctor_kind(), Some(CtorKind::Const)) + { + return FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_non_exhaustive, + help: None, + }; + }; let is_non_exhaustive = variant.is_field_list_non_exhaustive(); if is_non_exhaustive && !variant.def_id.is_local() { return FfiUnsafe { diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/auxiliary/types.rs b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/auxiliary/types.rs index d6251fcb768..4dc5932feab 100644 --- a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/auxiliary/types.rs +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/auxiliary/types.rs @@ -27,3 +27,14 @@ pub enum NonExhaustiveVariants { #[non_exhaustive] Tuple(u32), #[non_exhaustive] Struct { field: u32 } } + +// Note the absence of repr(C): it's not necessary, and recent C code can now use repr hints too. +#[repr(u32)] +#[non_exhaustive] +pub enum NonExhaustiveCLikeEnum { + One = 1, + Two = 2, + Three = 3, + Four = 4, + Five = 5, +} diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.rs b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.rs index 7a9b465bb56..c7f470fb787 100644 --- a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.rs +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.rs @@ -6,7 +6,10 @@ // This test checks that non-exhaustive types with `#[repr(C)]` from an extern crate are considered // improper. -use types::{NonExhaustiveEnum, NonExhaustiveVariants, NormalStruct, TupleStruct, UnitStruct}; +use types::{ + NonExhaustiveCLikeEnum, NonExhaustiveEnum, NonExhaustiveVariants, + NormalStruct, TupleStruct, UnitStruct, +}; extern "C" { pub fn non_exhaustive_enum(_: NonExhaustiveEnum); @@ -21,4 +24,9 @@ //~^ ERROR `extern` block uses type `NonExhaustiveVariants`, which is not FFI-safe } +// These should pass without remark, as they're C-compatible, despite being "non-exhaustive". +extern "C" { + pub fn non_exhaustive_c_compat_enum(_: NonExhaustiveCLikeEnum); +} + fn main() {} diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.stderr b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.stderr index 43c8e1015e6..afc3d3838ad 100644 --- a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.stderr +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.stderr @@ -1,5 +1,5 @@ error: `extern` block uses type `NonExhaustiveEnum`, which is not FFI-safe - --> $DIR/extern_crate_improper.rs:12:35 + --> $DIR/extern_crate_improper.rs:15:35 | LL | pub fn non_exhaustive_enum(_: NonExhaustiveEnum); | ^^^^^^^^^^^^^^^^^ not FFI-safe @@ -12,7 +12,7 @@ LL | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ error: `extern` block uses type `NormalStruct`, which is not FFI-safe - --> $DIR/extern_crate_improper.rs:14:44 + --> $DIR/extern_crate_improper.rs:17:44 | LL | pub fn non_exhaustive_normal_struct(_: NormalStruct); | ^^^^^^^^^^^^ not FFI-safe @@ -20,7 +20,7 @@ LL | pub fn non_exhaustive_normal_struct(_: NormalStruct); = note: this struct is non-exhaustive error: `extern` block uses type `UnitStruct`, which is not FFI-safe - --> $DIR/extern_crate_improper.rs:16:42 + --> $DIR/extern_crate_improper.rs:19:42 | LL | pub fn non_exhaustive_unit_struct(_: UnitStruct); | ^^^^^^^^^^ not FFI-safe @@ -28,7 +28,7 @@ LL | pub fn non_exhaustive_unit_struct(_: UnitStruct); = note: this struct is non-exhaustive error: `extern` block uses type `TupleStruct`, which is not FFI-safe - --> $DIR/extern_crate_improper.rs:18:43 + --> $DIR/extern_crate_improper.rs:21:43 | LL | pub fn non_exhaustive_tuple_struct(_: TupleStruct); | ^^^^^^^^^^^ not FFI-safe @@ -36,7 +36,7 @@ LL | pub fn non_exhaustive_tuple_struct(_: TupleStruct); = note: this struct is non-exhaustive error: `extern` block uses type `NonExhaustiveVariants`, which is not FFI-safe - --> $DIR/extern_crate_improper.rs:20:38 + --> $DIR/extern_crate_improper.rs:23:38 | LL | pub fn non_exhaustive_variant(_: NonExhaustiveVariants); | ^^^^^^^^^^^^^^^^^^^^^ not FFI-safe