Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mir_build: consider privacy when checking for irrefutable patterns #138001

Merged
merged 2 commits into from
Mar 20, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
@@ -42,7 +42,6 @@ use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeAndMut, TypeVisitableExt, VariantDef};
use rustc_middle::{bug, span_bug};
use rustc_session::lint;
use rustc_span::def_id::LOCAL_CRATE;
use rustc_span::{DUMMY_SP, Span, sym};
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_type_ir::elaborate;
@@ -789,7 +788,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {
_ => return Err(CastError::NonScalar),
};
if let ty::Adt(adt_def, _) = *self.expr_ty.kind()
&& adt_def.did().krate != LOCAL_CRATE
&& !adt_def.did().is_local()
&& adt_def.variants().iter().any(VariantDef::is_field_list_non_exhaustive)
{
return Err(CastError::ForeignNonExhaustiveAdt);
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
@@ -1966,7 +1966,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// Prohibit struct expressions when non-exhaustive flag is set.
let adt = adt_ty.ty_adt_def().expect("`check_struct_path` returned non-ADT type");
if !adt.did().is_local() && variant.is_field_list_non_exhaustive() {
if variant.field_list_has_applicable_non_exhaustive() {
self.dcx()
.emit_err(StructExprNonExhaustive { span: expr.span, what: adt.variant_descr() });
}
11 changes: 2 additions & 9 deletions compiler/rustc_hir_typeck/src/expr_use_visitor.rs
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@ use rustc_middle::hir::place::ProjectionKind;
pub use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, Projection};
use rustc_middle::mir::FakeReadCause;
use rustc_middle::ty::{
self, AdtKind, BorrowKind, Ty, TyCtxt, TypeFoldable, TypeVisitableExt as _, adjustment,
self, BorrowKind, Ty, TyCtxt, TypeFoldable, TypeVisitableExt as _, adjustment,
};
use rustc_middle::{bug, span_bug};
use rustc_span::{ErrorGuaranteed, Span};
@@ -1834,14 +1834,7 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
// to assume that more cases will be added to the variant in the future. This mean
// that we should handle non-exhaustive SingleVariant the same way we would handle
// a MultiVariant.
// If the variant is not local it must be defined in another crate.
let is_non_exhaustive = match def.adt_kind() {
AdtKind::Struct | AdtKind::Union => {
def.non_enum_variant().is_field_list_non_exhaustive()
}
AdtKind::Enum => def.is_variant_list_non_exhaustive(),
};
def.variants().len() > 1 || (!def.did().is_local() && is_non_exhaustive)
def.variants().len() > 1 || def.variant_list_has_applicable_non_exhaustive()
} else {
false
}
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
@@ -1722,7 +1722,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};

// Require `..` if struct has non_exhaustive attribute.
let non_exhaustive = variant.is_field_list_non_exhaustive() && !adt.did().is_local();
let non_exhaustive = variant.field_list_has_applicable_non_exhaustive();
if non_exhaustive && !has_rest_pat {
self.error_foreign_non_exhaustive_spat(pat, adt.variant_descr(), fields.is_empty());
}
12 changes: 4 additions & 8 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
@@ -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) {
14 changes: 3 additions & 11 deletions compiler/rustc_lint/src/types/improper_ctypes.rs
Original file line number Diff line number Diff line change
@@ -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()
}
11 changes: 11 additions & 0 deletions compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
@@ -329,11 +329,22 @@ impl<'tcx> AdtDef<'tcx> {
}

/// Returns `true` if the variant list of this ADT is `#[non_exhaustive]`.
///
/// Note that this function will return `true` even if the ADT has been
/// defined in the crate currently being compiled. If that's not what you
/// want, see [`Self::variant_list_has_applicable_non_exhaustive`].
#[inline]
pub fn is_variant_list_non_exhaustive(self) -> bool {
self.flags().contains(AdtFlags::IS_VARIANT_LIST_NON_EXHAUSTIVE)
}

/// Returns `true` if the variant list of this ADT is `#[non_exhaustive]`
/// and has been defined in another crate.
#[inline]
pub fn variant_list_has_applicable_non_exhaustive(self) -> bool {
self.is_variant_list_non_exhaustive() && !self.did().is_local()
}

/// Returns the kind of the ADT.
#[inline]
pub fn adt_kind(self) -> AdtKind {
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/inhabitedness/mod.rs
Original file line number Diff line number Diff line change
@@ -107,7 +107,7 @@ impl<'tcx> Ty<'tcx> {
// For now, unions are always considered inhabited
Adt(adt, _) if adt.is_union() => InhabitedPredicate::True,
// Non-exhaustive ADTs from other crates are always considered inhabited
Adt(adt, _) if adt.is_variant_list_non_exhaustive() && !adt.did().is_local() => {
Adt(adt, _) if adt.variant_list_has_applicable_non_exhaustive() => {
InhabitedPredicate::True
}
Never => InhabitedPredicate::False,
13 changes: 12 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
@@ -1206,12 +1206,23 @@ impl VariantDef {
}
}

/// Is this field list non-exhaustive?
/// Returns `true` if the field list of this variant is `#[non_exhaustive]`.
///
/// Note that this function will return `true` even if the type has been
/// defined in the crate currently being compiled. If that's not what you
/// want, see [`Self::field_list_has_applicable_non_exhaustive`].
#[inline]
pub fn is_field_list_non_exhaustive(&self) -> bool {
self.flags.intersects(VariantFlags::IS_FIELD_LIST_NON_EXHAUSTIVE)
}

/// Returns `true` if the field list of this variant is `#[non_exhaustive]`
/// and the type has been defined in another crate.
#[inline]
pub fn field_list_has_applicable_non_exhaustive(&self) -> bool {
self.is_field_list_non_exhaustive() && !self.def_id.is_local()
}

/// Computes the `Ident` of this variant by looking up the `Span`
pub fn ident(&self, tcx: TyCtxt<'_>) -> Ident {
Ident::new(self.name, tcx.def_ident_span(self.def_id).unwrap())
12 changes: 6 additions & 6 deletions compiler/rustc_mir_build/src/builder/matches/match_pair.rs
Original file line number Diff line number Diff line change
@@ -273,12 +273,12 @@ impl<'tcx> MatchPairTree<'tcx> {

let irrefutable = adt_def.variants().iter_enumerated().all(|(i, v)| {
i == variant_index
|| !v
.inhabited_predicate(cx.tcx, adt_def)
.instantiate(cx.tcx, args)
.apply_ignore_module(cx.tcx, cx.infcx.typing_env(cx.param_env))
}) && (adt_def.did().is_local()
|| !adt_def.is_variant_list_non_exhaustive());
|| !v.inhabited_predicate(cx.tcx, adt_def).instantiate(cx.tcx, args).apply(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

cx.tcx,
cx.infcx.typing_env(cx.param_env),
cx.def_id.into(),
)
}) && !adt_def.variant_list_has_applicable_non_exhaustive();
if irrefutable { None } else { Some(TestCase::Variant { adt_def, variant_index }) }
}

6 changes: 3 additions & 3 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
@@ -613,9 +613,9 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for NonExhaustivePatternsTypeNo
diag.span_note(span, fluent::mir_build_def_note);
}

let is_variant_list_non_exhaustive = matches!(self.ty.kind(),
ty::Adt(def, _) if def.is_variant_list_non_exhaustive() && !def.did().is_local());
if is_variant_list_non_exhaustive {
let is_non_exhaustive = matches!(self.ty.kind(),
ty::Adt(def, _) if def.variant_list_has_applicable_non_exhaustive());
if is_non_exhaustive {
diag.note(fluent::mir_build_non_exhaustive_type_note);
} else {
diag.note(fluent::mir_build_type_note);
4 changes: 1 addition & 3 deletions compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
@@ -150,9 +150,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
/// Returns whether the given type is an enum from another crate declared `#[non_exhaustive]`.
pub fn is_foreign_non_exhaustive_enum(&self, ty: RevealedTy<'tcx>) -> bool {
match ty.kind() {
ty::Adt(def, ..) => {
def.is_enum() && def.is_variant_list_non_exhaustive() && !def.did().is_local()
}
ty::Adt(def, ..) => def.variant_list_has_applicable_non_exhaustive(),
_ => false,
}
}
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/default.rs
Original file line number Diff line number Diff line change
@@ -134,7 +134,7 @@ impl<'tcx> LateLintPass<'tcx> for Default {
&& let ty::Adt(adt, args) = *binding_type.kind()
&& adt.is_struct()
&& let variant = adt.non_enum_variant()
&& (adt.did().is_local() || !variant.is_field_list_non_exhaustive())
&& !variant.field_list_has_applicable_non_exhaustive()
&& let module_did = cx.tcx.parent_module(stmt.hir_id)
&& variant
.fields
5 changes: 3 additions & 2 deletions src/tools/clippy/clippy_lints/src/needless_update.rs
Original file line number Diff line number Diff line change
@@ -54,8 +54,9 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessUpdate {
if let ExprKind::Struct(_, fields, StructTailExpr::Base(base)) = expr.kind {
let ty = cx.typeck_results().expr_ty(expr);
if let ty::Adt(def, _) = ty.kind() {
if fields.len() == def.non_enum_variant().fields.len()
&& !def.variant(0_usize.into()).is_field_list_non_exhaustive()
let variant = def.non_enum_variant();
if fields.len() == variant.fields.len()
&& !variant.is_field_list_non_exhaustive()
{
span_lint(
cx,
Original file line number Diff line number Diff line change
@@ -51,7 +51,7 @@ impl LateLintPass<'_> for UnneededStructPattern {
let variant = cx.tcx.adt_def(enum_did).variant_with_id(did);

let has_only_fields_brackets = variant.ctor.is_some() && variant.fields.is_empty();
let non_exhaustive_activated = !variant.def_id.is_local() && variant.is_field_list_non_exhaustive();
let non_exhaustive_activated = variant.field_list_has_applicable_non_exhaustive();
if !has_only_fields_brackets || non_exhaustive_activated {
return;
}
44 changes: 44 additions & 0 deletions tests/ui/match/privately-uninhabited-issue-137999.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//@ edition:2024
//@ check-fail

mod m {
enum Void {}

pub struct Internal {
_v: Void,
}

pub enum Test {
A(u32, u32),
B(Internal),
}
}

use m::Test;

pub fn f1(x: &mut Test) {
let r1: &mut u32 = match x {
Test::A(a, _) => a,
_ => todo!(),
};

let r2: &mut u32 = match x { //~ ERROR cannot use `*x` because it was mutably borrowed
Test::A(_, b) => b,
_ => todo!(),
};

let _ = *r1;
let _ = *r2;
}

pub fn f2(x: &mut Test) {
let r = &mut *x;
match x { //~ ERROR cannot use `*x` because it was mutably borrowed
Test::A(_, _) => {}
_ => {}
}

let _ = r;
}

fn main() {}
26 changes: 26 additions & 0 deletions tests/ui/match/privately-uninhabited-issue-137999.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error[E0503]: cannot use `*x` because it was mutably borrowed
--> $DIR/privately-uninhabited-issue-137999.rs:25:30
|
LL | Test::A(a, _) => a,
| - `x.0` is borrowed here
...
LL | let r2: &mut u32 = match x {
| ^ use of borrowed `x.0`
...
LL | let _ = *r1;
| --- borrow later used here

error[E0503]: cannot use `*x` because it was mutably borrowed
--> $DIR/privately-uninhabited-issue-137999.rs:36:11
|
LL | let r = &mut *x;
| ------- `*x` is borrowed here
LL | match x {
| ^ use of borrowed `*x`
...
LL | let _ = r;
| - borrow later used here

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0503`.
Loading