From be01e28dceb5e8e32bb1c97f3be5c5488eed8f4f Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 6 Feb 2024 02:35:59 +0100 Subject: [PATCH 1/6] Push down the decision to skip fields --- compiler/rustc_pattern_analysis/src/rustc.rs | 38 +++++++++++--------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 5b62731e20210..1b33ef5472eaa 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -165,13 +165,14 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { &self, ty: RevealedTy<'tcx>, variant: &'tcx VariantDef, - ) -> impl Iterator)> + Captures<'p> + Captures<'_> { + ) -> impl Iterator, bool)> + 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. + // Whether we must avoid matching 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)| { + variant.fields.iter().enumerate().map(move |(i, field)| { let ty = field.ty(cx.tcx, args); // `field.ty()` doesn't normalize after instantiating. let ty = cx.tcx.normalize_erasing_regions(cx.param_env, ty); @@ -180,12 +181,9 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { || 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 skip = is_uninhabited && (!is_visible || is_non_exhaustive); + let ty = cx.reveal_opaque_ty(ty); + (FieldIdx::new(i), ty, skip) }) } @@ -229,7 +227,10 @@ 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); + let tys = cx + .list_variant_nonhidden_fields(ty, variant) + .filter(|(_, _, skip)| !skip) + .map(|(_, ty, _)| ty); cx.dropless_arena.alloc_from_iter(tys) } } @@ -276,7 +277,9 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } else { let variant = &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); - self.list_variant_nonhidden_fields(ty, variant).count() + self.list_variant_nonhidden_fields(ty, variant) + .filter(|(_, _, skip)| !skip) + .count() } } _ => bug!("Unexpected type for constructor `{ctor:?}`: {ty:?}"), @@ -523,12 +526,14 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { // For each field in the variant, we store the relevant index into `self.fields` if any. let mut field_id_to_id: Vec> = (0..variant.fields.len()).map(|_| None).collect(); - let tys = cx.list_variant_nonhidden_fields(ty, variant).enumerate().map( - |(i, (field, ty))| { + let tys = cx + .list_variant_nonhidden_fields(ty, variant) + .filter(|(_, _, skip)| !skip) + .enumerate() + .map(|(i, (field, ty, _))| { field_id_to_id[field.index()] = Some(i); ty - }, - ); + }); fields = tys.map(|ty| DeconstructedPat::wildcard(ty)).collect(); for pat in subpatterns { if let Some(i) = field_id_to_id[pat.field.index()] { @@ -778,8 +783,9 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { let variant = &adt_def.variant(variant_index); let subpatterns = cx .list_variant_nonhidden_fields(*pat.ty(), variant) + .filter(|(_, _, skip)| !skip) .zip(subpatterns) - .map(|((field, _ty), pattern)| FieldPat { field, pattern }) + .map(|((field, _ty, _), pattern)| FieldPat { field, pattern }) .collect(); if adt_def.is_enum() { From ab06037269da9c5fc83083fb7d9d9638294e3d63 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 6 Feb 2024 02:44:48 +0100 Subject: [PATCH 2/6] Push the decision to skip fields further down --- compiler/rustc_pattern_analysis/src/lib.rs | 10 +++++++--- compiler/rustc_pattern_analysis/src/pat.rs | 8 ++++++-- compiler/rustc_pattern_analysis/src/rustc.rs | 16 +++++++++------- .../rustc_pattern_analysis/src/usefulness.rs | 6 ++++-- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 164dc36b679e7..0b4bc77a97639 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -82,6 +82,10 @@ use crate::usefulness::{compute_match_usefulness, ValidityConstraint}; pub trait Captures<'a> {} impl<'a, T: ?Sized> Captures<'a> for T {} +/// `bool` newtype that indicates whether we should skip this field during analysis. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct SkipField(pub bool); + /// Context that provides type information about constructors. /// /// Most of the crate is parameterized on a type that implements this trait. @@ -105,13 +109,13 @@ pub trait TypeCx: Sized + fmt::Debug { /// The number of fields for this constructor. fn ctor_arity(&self, ctor: &Constructor, 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()`-many + /// fields that are not skipped. fn ctor_sub_tys<'a>( &'a self, ctor: &'a Constructor, ty: &'a Self::Ty, - ) -> impl Iterator + ExactSizeIterator + Captures<'a>; + ) -> impl Iterator + ExactSizeIterator + Captures<'a>; /// The set of all the constructors for `ty`. /// diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index d9b2b31643d68..3e89a894419cf 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -5,7 +5,7 @@ use std::fmt; use smallvec::{smallvec, SmallVec}; use crate::constructor::{Constructor, Slice, SliceKind}; -use crate::TypeCx; +use crate::{SkipField, TypeCx}; use self::Constructor::*; @@ -300,7 +300,11 @@ impl WitnessPat { /// 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, 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(|(_, SkipField(skip))| !skip) + .map(|(ty, _)| Self::wildcard(ty)) + .collect(); Self::new(ctor, fields, ty) } diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 1b33ef5472eaa..8d55dcf2056f0 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -18,7 +18,7 @@ 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, SkipField, TypeCx}; use crate::constructor::Constructor::*; @@ -208,12 +208,15 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { &'a self, ctor: &'a Constructor<'p, 'tcx>, ty: RevealedTy<'tcx>, - ) -> impl Iterator> + ExactSizeIterator + Captures<'a> { + ) -> impl Iterator, SkipField)> + ExactSizeIterator + Captures<'a> + { fn reveal_and_alloc<'a, 'tcx>( cx: &'a RustcMatchCheckCtxt<'_, 'tcx>, iter: impl Iterator>, - ) -> &'a [RevealedTy<'tcx>] { - cx.dropless_arena.alloc_from_iter(iter.map(|ty| cx.reveal_opaque_ty(ty))) + ) -> &'a [(RevealedTy<'tcx>, SkipField)] { + cx.dropless_arena.alloc_from_iter( + iter.map(|ty| cx.reveal_opaque_ty(ty)).map(|ty| (ty, SkipField(false))), + ) } let cx = self; let slice = match ctor { @@ -229,8 +232,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); let tys = cx .list_variant_nonhidden_fields(ty, variant) - .filter(|(_, _, skip)| !skip) - .map(|(_, ty, _)| ty); + .map(|(_, ty, skip)| (ty, SkipField(skip))); cx.dropless_arena.alloc_from_iter(tys) } } @@ -872,7 +874,7 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> { &'a self, ctor: &'a crate::constructor::Constructor, ty: &'a Self::Ty, - ) -> impl Iterator + ExactSizeIterator + Captures<'a> { + ) -> impl Iterator + ExactSizeIterator + Captures<'a> { self.ctor_sub_tys(ctor, *ty) } fn ctors_for_ty( diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index f672051be5af2..ec9f3bb0db962 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -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, SkipField, TypeCx}; use self::ValidityConstraint::*; @@ -833,7 +833,9 @@ impl PlaceInfo { ) -> impl Iterator + 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 { + // Collect to keep the `ExactSizeIterator` bound. This is a temporary measure. + let tmp: Vec<_> = ctor_sub_tys.filter(|(_, SkipField(skip))| !skip).collect(); + tmp.into_iter().map(move |(ty, _)| PlaceInfo { ty, validity: ctor_sub_validity, is_scrutinee: false, From 4f7f06777bf67212aa960017ced0b911ab54bbf8 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 6 Feb 2024 02:53:37 +0100 Subject: [PATCH 3/6] Add special `Skip` constructor --- .../rustc_pattern_analysis/src/constructor.rs | 7 +++++ compiler/rustc_pattern_analysis/src/pat.rs | 4 ++- compiler/rustc_pattern_analysis/src/rustc.rs | 26 ++++--------------- 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 483986969d167..26b0f5fc45d9f 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -688,6 +688,10 @@ pub enum Constructor { /// 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 that we should skip the column entirely. This is used + /// when a private field is empty, so that we don't observe its emptiness. Only used for + /// specialization. + Skip, } impl Clone for Constructor { @@ -709,6 +713,7 @@ impl Clone for Constructor { Constructor::NonExhaustive => Constructor::NonExhaustive, Constructor::Hidden => Constructor::Hidden, Constructor::Missing => Constructor::Missing, + Constructor::Skip => Constructor::Skip, } } } @@ -763,6 +768,8 @@ impl Constructor { } // Wildcards cover anything (_, Wildcard) => true, + // `Skip` skips everything. + (Skip, _) => true, // Only a wildcard pattern can match these special constructors. (Missing { .. } | NonExhaustive | Hidden, _) => false, diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 3e89a894419cf..642ab74b8b992 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -84,6 +84,8 @@ impl DeconstructedPat { match (&self.ctor, other_ctor) { // Return a wildcard for each field of `other_ctor`. (Wildcard, _) => wildcard_sub_tys(), + // Skip this column. + (_, Skip) => 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. @@ -192,7 +194,7 @@ impl fmt::Debug for DeconstructedPat { } Ok(()) } - Wildcard | Missing { .. } | NonExhaustive | Hidden => write!(f, "_ : {:?}", pat.ty()), + Wildcard | Missing | NonExhaustive | Hidden | Skip => write!(f, "_ : {:?}", pat.ty()), } } } diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 8d55dcf2056f0..3b68dd503ad86 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -249,16 +249,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 | Skip | Wildcard => &[], Or => { bug!("called `Fields::wildcards` on an `Or` ctor") } @@ -288,16 +280,8 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { }, 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 | Skip | Wildcard => 0, Or => bug!("The `Or` constructor doesn't have a fixed arity"), } } @@ -838,7 +822,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } } &Str(value) => PatKind::Constant { value }, - Wildcard | NonExhaustive | Hidden => PatKind::Wild, + Wildcard | NonExhaustive | Hidden | Skip => 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`" From ea381663900b90c0f78c6a64cd5e0b1876047714 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 6 Feb 2024 03:50:22 +0100 Subject: [PATCH 4/6] Don't filter out skipped fields --- compiler/rustc_pattern_analysis/src/lib.rs | 3 +-- compiler/rustc_pattern_analysis/src/pat.rs | 5 ---- compiler/rustc_pattern_analysis/src/rustc.rs | 15 ++++------- .../rustc_pattern_analysis/src/usefulness.rs | 27 +++++++++++++++---- 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 0b4bc77a97639..e58e322a70dbb 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -109,8 +109,7 @@ pub trait TypeCx: Sized + fmt::Debug { /// The number of fields for this constructor. fn ctor_arity(&self, ctor: &Constructor, ty: &Self::Ty) -> usize; - /// The types of the fields for this constructor. The result must contain `ctor_arity()`-many - /// fields that are not skipped. + /// 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, diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 642ab74b8b992..8e1c22b92c8cf 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -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 { ctor: Constructor, fields: Vec>, diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 3b68dd503ad86..4e90d1a74064c 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -271,9 +271,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } else { let variant = &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); - self.list_variant_nonhidden_fields(ty, variant) - .filter(|(_, _, skip)| !skip) - .count() + self.list_variant_nonhidden_fields(ty, variant).count() } } _ => bug!("Unexpected type for constructor `{ctor:?}`: {ty:?}"), @@ -512,14 +510,12 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { // For each field in the variant, we store the relevant index into `self.fields` if any. let mut field_id_to_id: Vec> = (0..variant.fields.len()).map(|_| None).collect(); - let tys = cx - .list_variant_nonhidden_fields(ty, variant) - .filter(|(_, _, skip)| !skip) - .enumerate() - .map(|(i, (field, ty, _))| { + 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(); for pat in subpatterns { if let Some(i) = field_id_to_id[pat.field.index()] { @@ -769,7 +765,6 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { let variant = &adt_def.variant(variant_index); let subpatterns = cx .list_variant_nonhidden_fields(*pat.ty(), variant) - .filter(|(_, _, skip)| !skip) .zip(subpatterns) .map(|((field, _ty, _), pattern)| FieldPat { field, pattern }) .collect(); diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index ec9f3bb0db962..99d1f46e80430 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -817,6 +817,9 @@ impl fmt::Display for ValidityConstraint { struct PlaceInfo { /// The type of the place. ty: Cx::Ty, + /// Whether we must skip this field during analysis. This is used when a private field is empty, + /// so that we don't observe its emptiness. + skip: SkipField, /// Whether the place is known to contain valid data. validity: ValidityConstraint, /// Whether the place is the scrutinee itself or a subplace of it. @@ -833,10 +836,9 @@ impl PlaceInfo { ) -> impl Iterator + ExactSizeIterator + Captures<'a> { let ctor_sub_tys = cx.ctor_sub_tys(ctor, &self.ty); let ctor_sub_validity = self.validity.specialize(ctor); - // Collect to keep the `ExactSizeIterator` bound. This is a temporary measure. - let tmp: Vec<_> = ctor_sub_tys.filter(|(_, SkipField(skip))| !skip).collect(); - tmp.into_iter().map(move |(ty, _)| PlaceInfo { + ctor_sub_tys.map(move |(ty, skip)| PlaceInfo { ty, + skip, validity: ctor_sub_validity, is_scrutinee: false, }) @@ -858,6 +860,11 @@ impl PlaceInfo { where Cx: 'a, { + if matches!(self.skip, SkipField(true)) { + // Skip the whole column + return Ok((smallvec![Constructor::Skip], vec![])); + } + let ctors_for_ty = cx.ctors_for_ty(&self.ty)?; // We treat match scrutinees of type `!` or `EmptyEnum` differently. @@ -916,7 +923,12 @@ impl PlaceInfo { impl Clone for PlaceInfo { fn clone(&self) -> Self { - Self { ty: self.ty.clone(), validity: self.validity, is_scrutinee: self.is_scrutinee } + Self { + ty: self.ty.clone(), + skip: self.skip, + validity: self.validity, + is_scrutinee: self.is_scrutinee, + } } } @@ -1123,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, + skip: SkipField(false), + validity: scrut_validity, + is_scrutinee: true, + }; let mut matrix = Matrix { rows: Vec::with_capacity(arms.len()), place_info: smallvec![place_info], From 39441e4cdd46c61be6b86e4bfe352d1e7d5af6fb Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 6 Feb 2024 03:12:21 +0100 Subject: [PATCH 5/6] Simplify --- compiler/rustc_pattern_analysis/src/rustc.rs | 80 +++++++++----------- 1 file changed, 34 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 4e90d1a74064c..4f5f03838901b 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -10,7 +10,7 @@ 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}; @@ -158,32 +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, bool)> + Captures<'p> + Captures<'_> + ) -> impl Iterator)> + Captures<'p> + Captures<'_> { - let cx = self; - let ty::Adt(adt, args) = ty.kind() else { bug!() }; - // Whether we must avoid matching the fields of this variant exhaustively. - let is_non_exhaustive = variant.is_field_list_non_exhaustive() && !adt.did().is_local(); - - variant.fields.iter().enumerate().map(move |(i, field)| { - let ty = field.ty(cx.tcx, args); + 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); - - let skip = is_uninhabited && (!is_visible || is_non_exhaustive); - let ty = cx.reveal_opaque_ty(ty); - (FieldIdx::new(i), ty, skip) + let ty = self.tcx.normalize_erasing_regions(self.param_env, ty); + let ty = self.reveal_opaque_ty(ty); + (field, ty) }) } @@ -230,9 +217,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, skip)| (ty, SkipField(skip))); + + // 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, SkipField(skip)) + }); cx.dropless_arena.alloc_from_iter(tys) } } @@ -269,9 +268,8 @@ 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:?}"), @@ -507,20 +505,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> = - (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), @@ -762,11 +752,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() { From c918893b63022c1d810a71e8b7fa211b6aecd782 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 28 Feb 2024 17:56:01 +0100 Subject: [PATCH 6/6] Rename `Skip` to `PrivateUninhabited` --- .../rustc_pattern_analysis/src/constructor.rs | 13 +++++------ compiler/rustc_pattern_analysis/src/lib.rs | 7 +++--- compiler/rustc_pattern_analysis/src/pat.rs | 10 ++++---- compiler/rustc_pattern_analysis/src/rustc.rs | 23 +++++++++++-------- .../rustc_pattern_analysis/src/usefulness.rs | 18 +++++++-------- 5 files changed, 38 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 26b0f5fc45d9f..1286022fe4d1c 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -688,10 +688,9 @@ pub enum Constructor { /// 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 that we should skip the column entirely. This is used - /// when a private field is empty, so that we don't observe its emptiness. Only used for - /// specialization. - Skip, + /// 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 Clone for Constructor { @@ -713,7 +712,7 @@ impl Clone for Constructor { Constructor::NonExhaustive => Constructor::NonExhaustive, Constructor::Hidden => Constructor::Hidden, Constructor::Missing => Constructor::Missing, - Constructor::Skip => Constructor::Skip, + Constructor::PrivateUninhabited => Constructor::PrivateUninhabited, } } } @@ -768,8 +767,8 @@ impl Constructor { } // Wildcards cover anything (_, Wildcard) => true, - // `Skip` skips everything. - (Skip, _) => true, + // `PrivateUninhabited` skips everything. + (PrivateUninhabited, _) => true, // Only a wildcard pattern can match these special constructors. (Missing { .. } | NonExhaustive | Hidden, _) => false, diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index e58e322a70dbb..d4b38d260e717 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -82,9 +82,10 @@ use crate::usefulness::{compute_match_usefulness, ValidityConstraint}; pub trait Captures<'a> {} impl<'a, T: ?Sized> Captures<'a> for T {} -/// `bool` newtype that indicates whether we should skip this field during analysis. +/// `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 SkipField(pub bool); +pub struct PrivateUninhabitedField(pub bool); /// Context that provides type information about constructors. /// @@ -114,7 +115,7 @@ pub trait TypeCx: Sized + fmt::Debug { &'a self, ctor: &'a Constructor, ty: &'a Self::Ty, - ) -> impl Iterator + ExactSizeIterator + Captures<'a>; + ) -> impl Iterator + ExactSizeIterator + Captures<'a>; /// The set of all the constructors for `ty`. /// diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 8e1c22b92c8cf..decbfa5c0cf4d 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -5,7 +5,7 @@ use std::fmt; use smallvec::{smallvec, SmallVec}; use crate::constructor::{Constructor, Slice, SliceKind}; -use crate::{SkipField, TypeCx}; +use crate::{PrivateUninhabitedField, TypeCx}; use self::Constructor::*; @@ -80,7 +80,7 @@ impl DeconstructedPat { // Return a wildcard for each field of `other_ctor`. (Wildcard, _) => wildcard_sub_tys(), // Skip this column. - (_, Skip) => smallvec![], + (_, 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. @@ -189,7 +189,9 @@ impl fmt::Debug for DeconstructedPat { } Ok(()) } - Wildcard | Missing | NonExhaustive | Hidden | Skip => write!(f, "_ : {:?}", pat.ty()), + Wildcard | Missing | NonExhaustive | Hidden | PrivateUninhabited => { + write!(f, "_ : {:?}", pat.ty()) + } } } } @@ -299,7 +301,7 @@ impl WitnessPat { pub(crate) fn wild_from_ctor(cx: &Cx, ctor: Constructor, ty: Cx::Ty) -> Self { let fields = cx .ctor_sub_tys(&ctor, &ty) - .filter(|(_, SkipField(skip))| !skip) + .filter(|(_, PrivateUninhabitedField(skip))| !skip) .map(|(ty, _)| Self::wildcard(ty)) .collect(); Self::new(ctor, fields, ty) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 4f5f03838901b..7a0562e12f159 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -18,7 +18,7 @@ use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT}; use crate::constructor::{ IntRange, MaybeInfiniteInt, OpaqueId, RangeEnd, Slice, SliceKind, VariantVisibility, }; -use crate::{errors, Captures, SkipField, TypeCx}; +use crate::{errors, Captures, PrivateUninhabitedField, TypeCx}; use crate::constructor::Constructor::*; @@ -195,14 +195,16 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { &'a self, ctor: &'a Constructor<'p, 'tcx>, ty: RevealedTy<'tcx>, - ) -> impl Iterator, SkipField)> + ExactSizeIterator + Captures<'a> - { + ) -> impl Iterator, PrivateUninhabitedField)> + + ExactSizeIterator + + Captures<'a> { fn reveal_and_alloc<'a, 'tcx>( cx: &'a RustcMatchCheckCtxt<'_, 'tcx>, iter: impl Iterator>, - ) -> &'a [(RevealedTy<'tcx>, SkipField)] { + ) -> &'a [(RevealedTy<'tcx>, PrivateUninhabitedField)] { cx.dropless_arena.alloc_from_iter( - iter.map(|ty| cx.reveal_opaque_ty(ty)).map(|ty| (ty, SkipField(false))), + iter.map(|ty| cx.reveal_opaque_ty(ty)) + .map(|ty| (ty, PrivateUninhabitedField(false))), ) } let cx = self; @@ -230,7 +232,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { || cx.tcx.features().min_exhaustive_patterns) && cx.is_uninhabited(*ty); let skip = is_uninhabited && (!is_visible || is_non_exhaustive); - (ty, SkipField(skip)) + (ty, PrivateUninhabitedField(skip)) }); cx.dropless_arena.alloc_from_iter(tys) } @@ -249,7 +251,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { _ => bug!("bad slice pattern {:?} {:?}", ctor, ty), }, Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..) - | NonExhaustive | Hidden | Missing | Skip | Wildcard => &[], + | NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => &[], Or => { bug!("called `Fields::wildcards` on an `Or` ctor") } @@ -277,7 +279,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { Ref => 1, Slice(slice) => slice.arity(), Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..) - | NonExhaustive | Hidden | Missing | Skip | Wildcard => 0, + | NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => 0, Or => bug!("The `Or` constructor doesn't have a fixed arity"), } } @@ -805,7 +807,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } } &Str(value) => PatKind::Constant { value }, - Wildcard | NonExhaustive | Hidden | Skip => 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`" @@ -841,7 +843,8 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> { &'a self, ctor: &'a crate::constructor::Constructor, ty: &'a Self::Ty, - ) -> impl Iterator + ExactSizeIterator + Captures<'a> { + ) -> impl Iterator + ExactSizeIterator + Captures<'a> + { self.ctor_sub_tys(ctor, *ty) } fn ctors_for_ty( diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 99d1f46e80430..bbe02f94c0a02 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -716,7 +716,7 @@ use std::fmt; use crate::constructor::{Constructor, ConstructorSet, IntRange}; use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat}; -use crate::{Captures, MatchArm, SkipField, TypeCx}; +use crate::{Captures, MatchArm, PrivateUninhabitedField, TypeCx}; use self::ValidityConstraint::*; @@ -817,9 +817,9 @@ impl fmt::Display for ValidityConstraint { struct PlaceInfo { /// The type of the place. ty: Cx::Ty, - /// Whether we must skip this field during analysis. This is used when a private field is empty, + /// Whether the place is a private uninhabited field. If so we skip this field during analysis /// so that we don't observe its emptiness. - skip: SkipField, + 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. @@ -836,9 +836,9 @@ impl PlaceInfo { ) -> impl Iterator + 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, skip)| PlaceInfo { + ctor_sub_tys.map(move |(ty, PrivateUninhabitedField(private_uninhabited))| PlaceInfo { ty, - skip, + private_uninhabited, validity: ctor_sub_validity, is_scrutinee: false, }) @@ -860,9 +860,9 @@ impl PlaceInfo { where Cx: 'a, { - if matches!(self.skip, SkipField(true)) { + if self.private_uninhabited { // Skip the whole column - return Ok((smallvec![Constructor::Skip], vec![])); + return Ok((smallvec![Constructor::PrivateUninhabited], vec![])); } let ctors_for_ty = cx.ctors_for_ty(&self.ty)?; @@ -925,7 +925,7 @@ impl Clone for PlaceInfo { fn clone(&self) -> Self { Self { ty: self.ty.clone(), - skip: self.skip, + private_uninhabited: self.private_uninhabited, validity: self.validity, is_scrutinee: self.is_scrutinee, } @@ -1137,7 +1137,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { ) -> Self { let place_info = PlaceInfo { ty: scrut_ty, - skip: SkipField(false), + private_uninhabited: false, validity: scrut_validity, is_scrutinee: true, };