Rollup merge of #138001 - meithecatte:privately-uninhabited, r=Nadrieril

mir_build: consider privacy when checking for irrefutable patterns

This PR fixes #137999.

Note that, since this makes the compiler reject code that was previously accepted, it will probably need a crater run.

I include a commit that factors out a common code pattern into a helper function, purely because the fact that this was repeated all over the place was bothering me. Let me know if I should split that into a separate PR instead.
This commit is contained in:
Matthias Krüger 2025-03-19 16:52:54 +01:00 committed by GitHub
commit 2ab69b898a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 121 additions and 50 deletions

View file

@ -1193,9 +1193,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
};
}
let is_non_exhaustive =
def.non_enum_variant().is_field_list_non_exhaustive();
if is_non_exhaustive && !def.did().is_local() {
if def.non_enum_variant().field_list_has_applicable_non_exhaustive() {
return FfiUnsafe {
ty,
reason: if def.is_struct() {
@ -1248,14 +1246,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
};
}
use improper_ctypes::{
check_non_exhaustive_variant, non_local_and_non_exhaustive,
};
use improper_ctypes::check_non_exhaustive_variant;
let non_local_def = non_local_and_non_exhaustive(def);
let non_exhaustive = def.variant_list_has_applicable_non_exhaustive();
// Check the contained variants.
let ret = def.variants().iter().try_for_each(|variant| {
check_non_exhaustive_variant(non_local_def, variant)
check_non_exhaustive_variant(non_exhaustive, variant)
.map_break(|reason| FfiUnsafe { ty, reason, help: None })?;
match self.check_variant_for_ffi(acc, ty, def, variant, args) {

View file

@ -15,13 +15,13 @@ use crate::fluent_generated as fluent;
/// 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,
non_exhaustive_variant_list: 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 {
if non_exhaustive_variant_list {
// 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)
@ -30,8 +30,7 @@ pub(crate) fn check_non_exhaustive_variant(
}
}
let non_exhaustive_variant_fields = variant.is_field_list_non_exhaustive();
if non_exhaustive_variant_fields && !variant.def_id.is_local() {
if variant.field_list_has_applicable_non_exhaustive() {
return ControlFlow::Break(fluent::lint_improper_ctypes_non_exhaustive_variant);
}
@ -42,10 +41,3 @@ 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()
}