diff options
5 files changed, 91 insertions, 26 deletions
diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index db4413149a4..e9c61a41d6d 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -18,6 +18,8 @@ use rustc_target::spec::abi::Abi as SpecAbi; use tracing::debug; use {rustc_ast as ast, rustc_hir as hir}; +mod improper_ctypes; + use crate::lints::{ AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion, AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad, @@ -983,15 +985,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // 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() @@ -1010,21 +1003,23 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { }; } + use improper_ctypes::{ + check_non_exhaustive_variant, non_local_and_non_exhaustive, + }; + + let non_local_def = non_local_and_non_exhaustive(def); // Check the contained variants. - for variant in def.variants() { - let is_non_exhaustive = variant.is_field_list_non_exhaustive(); - if is_non_exhaustive && !variant.def_id.is_local() { - return FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_non_exhaustive_variant, - help: None, - }; - } + let ret = def.variants().iter().try_for_each(|variant| { + check_non_exhaustive_variant(non_local_def, variant) + .map_break(|reason| FfiUnsafe { ty, reason, help: None })?; match self.check_variant_for_ffi(acc, ty, def, variant, args) { - FfiSafe => (), - r => return r, + FfiSafe => ControlFlow::Continue(()), + r => ControlFlow::Break(r), } + }); + if let ControlFlow::Break(result) = ret { + return result; } FfiSafe diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs new file mode 100644 index 00000000000..1030101c545 --- /dev/null +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -0,0 +1,51 @@ +use std::ops::ControlFlow; + +use rustc_errors::DiagMessage; +use rustc_hir::def::CtorKind; +use rustc_middle::ty; + +use crate::fluent_generated as fluent; + +/// Check a variant of a non-exhaustive enum for improper ctypes +/// +/// We treat `#[non_exhaustive] enum` as "ensure that code will compile if new variants are added". +/// This includes linting, on a best-effort basis. There are valid additions that are unlikely. +/// +/// Adding a data-carrying variant to an existing C-like enum that is passed to C is "unlikely", +/// so we don't need the lint to account for it. +/// e.g. going from enum Foo { A, B, C } to enum Foo { A, B, C, D(u32) }. +pub(crate) fn check_non_exhaustive_variant( + non_local_def: bool, + variant: &ty::VariantDef, +) -> ControlFlow<DiagMessage, ()> { + // 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 + if non_local_def { + // which is why we only warn about really_tagged_union reprs from https://rust.tf/rfc2195 + // with an enum like `#[repr(u8)] enum Enum { A(DataA), B(DataB), }` + // but exempt enums with unit ctors like C's (e.g. from rust-bindgen) + if variant_has_complex_ctor(variant) { + return ControlFlow::Break(fluent::lint_improper_ctypes_non_exhaustive); + } + } + + let non_exhaustive_variant_fields = variant.is_field_list_non_exhaustive(); + if non_exhaustive_variant_fields && !variant.def_id.is_local() { + return ControlFlow::Break(fluent::lint_improper_ctypes_non_exhaustive_variant); + } + + ControlFlow::Continue(()) +} + +fn variant_has_complex_ctor(variant: &ty::VariantDef) -> bool { + // CtorKind::Const means a "unit" ctor + !matches!(variant.ctor_kind(), Some(CtorKind::Const)) +} + +// 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 +pub(crate) fn non_local_and_non_exhaustive(def: ty::AdtDef<'_>) -> bool { + def.is_variant_list_non_exhaustive() && !def.did().is_local() +} 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 @@ extern crate types; // 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 @@ extern "C" { //~^ 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 |