diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index eef1f0d133ae4..48e9e78db1e6f 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -3,7 +3,6 @@ use std::ops::ControlFlow; 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}; @@ -19,6 +18,8 @@ use rustc_target::spec::abi::Abi as SpecAbi; use tracing::debug; use {rustc_ast as ast, rustc_attr as attr, rustc_hir as hir}; +mod improper_ctypes; + use crate::lints::{ AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion, AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad, @@ -1405,38 +1406,23 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { }; } - // 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(); + 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() { - // 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 { - 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 0000000000000..dc8a9704d166d --- /dev/null +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -0,0 +1,50 @@ +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 { + // 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, + // 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() +}