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

pattern_analysis: rework how we hide empty private fields #121000

Merged
merged 6 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
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
6 changes: 6 additions & 0 deletions compiler/rustc_pattern_analysis/src/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,9 @@ pub enum Constructor<Cx: TypeCx> {
/// Fake extra constructor for constructors that are not seen in the matrix, as explained at the
/// top of the file.
Missing,
/// Fake extra constructor that indicates and empty field that is private. When we encounter one
/// we skip the column entirely so we don't observe its emptiness. Only used for specialization.
PrivateUninhabited,
}

impl<Cx: TypeCx> Clone for Constructor<Cx> {
Expand All @@ -709,6 +712,7 @@ impl<Cx: TypeCx> Clone for Constructor<Cx> {
Constructor::NonExhaustive => Constructor::NonExhaustive,
Constructor::Hidden => Constructor::Hidden,
Constructor::Missing => Constructor::Missing,
Constructor::PrivateUninhabited => Constructor::PrivateUninhabited,
}
}
}
Expand Down Expand Up @@ -763,6 +767,8 @@ impl<Cx: TypeCx> Constructor<Cx> {
}
// Wildcards cover anything
(_, Wildcard) => true,
// `PrivateUninhabited` skips everything.
(PrivateUninhabited, _) => true,
// Only a wildcard pattern can match these special constructors.
(Missing { .. } | NonExhaustive | Hidden, _) => false,

Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_pattern_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ use crate::usefulness::{compute_match_usefulness, ValidityConstraint};
pub trait Captures<'a> {}
impl<'a, T: ?Sized> Captures<'a> for T {}

/// `bool` newtype that indicates whether this is a privately uninhabited field that we should skip
/// during analysis.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub struct PrivateUninhabitedField(pub bool);

/// Context that provides type information about constructors.
///
/// Most of the crate is parameterized on a type that implements this trait.
Expand All @@ -105,13 +110,12 @@ pub trait TypeCx: Sized + fmt::Debug {
/// The number of fields for this constructor.
fn ctor_arity(&self, ctor: &Constructor<Self>, ty: &Self::Ty) -> usize;

/// The types of the fields for this constructor. The result must have a length of
/// `ctor_arity()`.
/// The types of the fields for this constructor. The result must contain `ctor_arity()` fields.
fn ctor_sub_tys<'a>(
&'a self,
ctor: &'a Constructor<Self>,
ty: &'a Self::Ty,
) -> impl Iterator<Item = Self::Ty> + ExactSizeIterator + Captures<'a>;
) -> impl Iterator<Item = (Self::Ty, PrivateUninhabitedField)> + ExactSizeIterator + Captures<'a>;

/// The set of all the constructors for `ty`.
///
Expand Down
19 changes: 11 additions & 8 deletions compiler/rustc_pattern_analysis/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::fmt;
use smallvec::{smallvec, SmallVec};

use crate::constructor::{Constructor, Slice, SliceKind};
use crate::TypeCx;
use crate::{PrivateUninhabitedField, TypeCx};

use self::Constructor::*;

Expand All @@ -23,11 +23,6 @@ impl PatId {
/// Values and patterns can be represented as a constructor applied to some fields. This represents
/// a pattern in this form. A `DeconstructedPat` will almost always come from user input; the only
/// exception are some `Wildcard`s introduced during pattern lowering.
///
/// Note that the number of fields may not match the fields declared in the original struct/variant.
/// This happens if a private or `non_exhaustive` field is uninhabited, because the code mustn't
/// observe that it is uninhabited. In that case that field is not included in `fields`. Care must
/// be taken when converting to/from `thir::Pat`.
pub struct DeconstructedPat<Cx: TypeCx> {
ctor: Constructor<Cx>,
fields: Vec<DeconstructedPat<Cx>>,
Expand Down Expand Up @@ -84,6 +79,8 @@ impl<Cx: TypeCx> DeconstructedPat<Cx> {
match (&self.ctor, other_ctor) {
// Return a wildcard for each field of `other_ctor`.
(Wildcard, _) => wildcard_sub_tys(),
// Skip this column.
(_, PrivateUninhabited) => smallvec![],
// The only non-trivial case: two slices of different arity. `other_slice` is
// guaranteed to have a larger arity, so we fill the middle part with enough
// wildcards to reach the length of the new, larger slice.
Expand Down Expand Up @@ -192,7 +189,9 @@ impl<Cx: TypeCx> fmt::Debug for DeconstructedPat<Cx> {
}
Ok(())
}
Wildcard | Missing { .. } | NonExhaustive | Hidden => write!(f, "_ : {:?}", pat.ty()),
Wildcard | Missing | NonExhaustive | Hidden | PrivateUninhabited => {
write!(f, "_ : {:?}", pat.ty())
}
}
}
}
Expand Down Expand Up @@ -300,7 +299,11 @@ impl<Cx: TypeCx> WitnessPat<Cx> {
/// For example, if `ctor` is a `Constructor::Variant` for `Option::Some`, we get the pattern
/// `Some(_)`.
pub(crate) fn wild_from_ctor(cx: &Cx, ctor: Constructor<Cx>, ty: Cx::Ty) -> Self {
let fields = cx.ctor_sub_tys(&ctor, &ty).map(|ty| Self::wildcard(ty)).collect();
let fields = cx
.ctor_sub_tys(&ctor, &ty)
.filter(|(_, PrivateUninhabitedField(skip))| !skip)
.map(|(ty, _)| Self::wildcard(ty))
.collect();
Self::new(ctor, fields, ty)
}

Expand Down
124 changes: 51 additions & 73 deletions compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::{self, Const};
use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange, PatRangeBoundary};
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::{self, OpaqueTypeKey, Ty, TyCtxt, TypeVisitableExt, VariantDef};
use rustc_middle::ty::{self, FieldDef, OpaqueTypeKey, Ty, TyCtxt, TypeVisitableExt, VariantDef};
use rustc_session::lint;
use rustc_span::{ErrorGuaranteed, Span, DUMMY_SP};
use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT};

use crate::constructor::{
IntRange, MaybeInfiniteInt, OpaqueId, RangeEnd, Slice, SliceKind, VariantVisibility,
};
use crate::{errors, Captures, TypeCx};
use crate::{errors, Captures, PrivateUninhabitedField, TypeCx};

use crate::constructor::Constructor::*;

Expand Down Expand Up @@ -158,34 +158,19 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
}
}

// In the cases of either a `#[non_exhaustive]` field list or a non-public field, we hide
// uninhabited fields in order not to reveal the uninhabitedness of the whole variant.
// This lists the fields we keep along with their types.
pub(crate) fn list_variant_nonhidden_fields(
pub(crate) fn variant_sub_tys(
&self,
ty: RevealedTy<'tcx>,
variant: &'tcx VariantDef,
) -> impl Iterator<Item = (FieldIdx, RevealedTy<'tcx>)> + Captures<'p> + Captures<'_> {
let cx = self;
let ty::Adt(adt, args) = ty.kind() else { bug!() };
// Whether we must not match the fields of this variant exhaustively.
let is_non_exhaustive = variant.is_field_list_non_exhaustive() && !adt.did().is_local();

variant.fields.iter().enumerate().filter_map(move |(i, field)| {
let ty = field.ty(cx.tcx, args);
) -> impl Iterator<Item = (&'tcx FieldDef, RevealedTy<'tcx>)> + Captures<'p> + Captures<'_>
{
let ty::Adt(_, args) = ty.kind() else { bug!() };
variant.fields.iter().map(move |field| {
let ty = field.ty(self.tcx, args);
// `field.ty()` doesn't normalize after instantiating.
let ty = cx.tcx.normalize_erasing_regions(cx.param_env, ty);
let is_visible = adt.is_enum() || field.vis.is_accessible_from(cx.module, cx.tcx);
let is_uninhabited = (cx.tcx.features().exhaustive_patterns
|| cx.tcx.features().min_exhaustive_patterns)
&& cx.is_uninhabited(ty);

if is_uninhabited && (!is_visible || is_non_exhaustive) {
None
} else {
let ty = cx.reveal_opaque_ty(ty);
Some((FieldIdx::new(i), ty))
}
let ty = self.tcx.normalize_erasing_regions(self.param_env, ty);
let ty = self.reveal_opaque_ty(ty);
(field, ty)
})
}

Expand All @@ -210,12 +195,17 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
&'a self,
ctor: &'a Constructor<'p, 'tcx>,
ty: RevealedTy<'tcx>,
) -> impl Iterator<Item = RevealedTy<'tcx>> + ExactSizeIterator + Captures<'a> {
) -> impl Iterator<Item = (RevealedTy<'tcx>, PrivateUninhabitedField)>
+ ExactSizeIterator
+ Captures<'a> {
fn reveal_and_alloc<'a, 'tcx>(
cx: &'a RustcMatchCheckCtxt<'_, 'tcx>,
iter: impl Iterator<Item = Ty<'tcx>>,
) -> &'a [RevealedTy<'tcx>] {
cx.dropless_arena.alloc_from_iter(iter.map(|ty| cx.reveal_opaque_ty(ty)))
) -> &'a [(RevealedTy<'tcx>, PrivateUninhabitedField)] {
cx.dropless_arena.alloc_from_iter(
iter.map(|ty| cx.reveal_opaque_ty(ty))
.map(|ty| (ty, PrivateUninhabitedField(false))),
)
}
let cx = self;
let slice = match ctor {
Expand All @@ -229,7 +219,21 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
} else {
let variant =
&adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt));
let tys = cx.list_variant_nonhidden_fields(ty, variant).map(|(_, ty)| ty);

// In the cases of either a `#[non_exhaustive]` field list or a non-public
// field, we skip uninhabited fields in order not to reveal the
// uninhabitedness of the whole variant.
let is_non_exhaustive =
variant.is_field_list_non_exhaustive() && !adt.did().is_local();
let tys = cx.variant_sub_tys(ty, variant).map(|(field, ty)| {
let is_visible =
adt.is_enum() || field.vis.is_accessible_from(cx.module, cx.tcx);
let is_uninhabited = (cx.tcx.features().exhaustive_patterns
|| cx.tcx.features().min_exhaustive_patterns)
&& cx.is_uninhabited(*ty);
let skip = is_uninhabited && (!is_visible || is_non_exhaustive);
(ty, PrivateUninhabitedField(skip))
});
cx.dropless_arena.alloc_from_iter(tys)
}
}
Expand All @@ -246,16 +250,8 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
}
_ => bug!("bad slice pattern {:?} {:?}", ctor, ty),
},
Bool(..)
| IntRange(..)
| F32Range(..)
| F64Range(..)
| Str(..)
| Opaque(..)
| NonExhaustive
| Hidden
| Missing { .. }
| Wildcard => &[],
Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..)
| NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => &[],
Or => {
bug!("called `Fields::wildcards` on an `Or` ctor")
}
Expand All @@ -274,25 +270,16 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
// patterns. If we're here we can assume this is a box pattern.
1
} else {
let variant =
&adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt));
self.list_variant_nonhidden_fields(ty, variant).count()
let variant_idx = RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt);
adt.variant(variant_idx).fields.len()
}
}
_ => bug!("Unexpected type for constructor `{ctor:?}`: {ty:?}"),
},
Ref => 1,
Slice(slice) => slice.arity(),
Bool(..)
| IntRange(..)
| F32Range(..)
| F64Range(..)
| Str(..)
| Opaque(..)
| NonExhaustive
| Hidden
| Missing { .. }
| Wildcard => 0,
Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..)
| NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => 0,
Or => bug!("The `Or` constructor doesn't have a fixed arity"),
}
}
Expand Down Expand Up @@ -520,20 +507,12 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
};
let variant =
&adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt));
// For each field in the variant, we store the relevant index into `self.fields` if any.
let mut field_id_to_id: Vec<Option<usize>> =
(0..variant.fields.len()).map(|_| None).collect();
let tys = cx.list_variant_nonhidden_fields(ty, variant).enumerate().map(
|(i, (field, ty))| {
field_id_to_id[field.index()] = Some(i);
ty
},
);
fields = tys.map(|ty| DeconstructedPat::wildcard(ty)).collect();
fields = cx
.variant_sub_tys(ty, variant)
.map(|(_, ty)| DeconstructedPat::wildcard(ty))
.collect();
for pat in subpatterns {
if let Some(i) = field_id_to_id[pat.field.index()] {
fields[i] = self.lower_pat(&pat.pattern);
}
fields[pat.field.index()] = self.lower_pat(&pat.pattern);
}
}
_ => bug!("pattern has unexpected type: pat: {:?}, ty: {:?}", pat, ty),
Expand Down Expand Up @@ -775,11 +754,9 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
ty::Adt(adt_def, args) => {
let variant_index =
RustcMatchCheckCtxt::variant_index_for_adt(&pat.ctor(), *adt_def);
let variant = &adt_def.variant(variant_index);
let subpatterns = cx
.list_variant_nonhidden_fields(*pat.ty(), variant)
.zip(subpatterns)
.map(|((field, _ty), pattern)| FieldPat { field, pattern })
let subpatterns = subpatterns
.enumerate()
.map(|(i, pattern)| FieldPat { field: FieldIdx::new(i), pattern })
.collect();

if adt_def.is_enum() {
Expand Down Expand Up @@ -830,7 +807,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
}
}
&Str(value) => PatKind::Constant { value },
Wildcard | NonExhaustive | Hidden => PatKind::Wild,
Wildcard | NonExhaustive | Hidden | PrivateUninhabited => PatKind::Wild,
Missing { .. } => bug!(
"trying to convert a `Missing` constructor into a `Pat`; this is probably a bug,
`Missing` should have been processed in `apply_constructors`"
Expand Down Expand Up @@ -866,7 +843,8 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> {
&'a self,
ctor: &'a crate::constructor::Constructor<Self>,
ty: &'a Self::Ty,
) -> impl Iterator<Item = Self::Ty> + ExactSizeIterator + Captures<'a> {
) -> impl Iterator<Item = (Self::Ty, PrivateUninhabitedField)> + ExactSizeIterator + Captures<'a>
{
self.ctor_sub_tys(ctor, *ty)
}
fn ctors_for_ty(
Expand Down
27 changes: 23 additions & 4 deletions compiler/rustc_pattern_analysis/src/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ use std::fmt;

use crate::constructor::{Constructor, ConstructorSet, IntRange};
use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat};
use crate::{Captures, MatchArm, TypeCx};
use crate::{Captures, MatchArm, PrivateUninhabitedField, TypeCx};

use self::ValidityConstraint::*;

Expand Down Expand Up @@ -817,6 +817,9 @@ impl fmt::Display for ValidityConstraint {
struct PlaceInfo<Cx: TypeCx> {
/// The type of the place.
ty: Cx::Ty,
/// Whether the place is a private uninhabited field. If so we skip this field during analysis
/// so that we don't observe its emptiness.
private_uninhabited: bool,
/// Whether the place is known to contain valid data.
validity: ValidityConstraint,
/// Whether the place is the scrutinee itself or a subplace of it.
Expand All @@ -833,8 +836,9 @@ impl<Cx: TypeCx> PlaceInfo<Cx> {
) -> impl Iterator<Item = Self> + ExactSizeIterator + Captures<'a> {
let ctor_sub_tys = cx.ctor_sub_tys(ctor, &self.ty);
let ctor_sub_validity = self.validity.specialize(ctor);
ctor_sub_tys.map(move |ty| PlaceInfo {
ctor_sub_tys.map(move |(ty, PrivateUninhabitedField(private_uninhabited))| PlaceInfo {
ty,
private_uninhabited,
validity: ctor_sub_validity,
is_scrutinee: false,
})
Expand All @@ -856,6 +860,11 @@ impl<Cx: TypeCx> PlaceInfo<Cx> {
where
Cx: 'a,
{
if self.private_uninhabited {
// Skip the whole column
return Ok((smallvec![Constructor::PrivateUninhabited], vec![]));
}

let ctors_for_ty = cx.ctors_for_ty(&self.ty)?;

// We treat match scrutinees of type `!` or `EmptyEnum` differently.
Expand Down Expand Up @@ -914,7 +923,12 @@ impl<Cx: TypeCx> PlaceInfo<Cx> {

impl<Cx: TypeCx> Clone for PlaceInfo<Cx> {
fn clone(&self) -> Self {
Self { ty: self.ty.clone(), validity: self.validity, is_scrutinee: self.is_scrutinee }
Self {
ty: self.ty.clone(),
private_uninhabited: self.private_uninhabited,
validity: self.validity,
is_scrutinee: self.is_scrutinee,
}
}
}

Expand Down Expand Up @@ -1121,7 +1135,12 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
scrut_ty: Cx::Ty,
scrut_validity: ValidityConstraint,
) -> Self {
let place_info = PlaceInfo { ty: scrut_ty, validity: scrut_validity, is_scrutinee: true };
let place_info = PlaceInfo {
ty: scrut_ty,
private_uninhabited: false,
validity: scrut_validity,
is_scrutinee: true,
};
let mut matrix = Matrix {
rows: Vec::with_capacity(arms.len()),
place_info: smallvec![place_info],
Expand Down
Loading