Skip to content

Commit b5ae3af

Browse files
rust-timerrust-timer
authored
Unrolled build for #121000
Rollup merge of #121000 - Nadrieril:keep_all_fields, r=compiler-errors pattern_analysis: rework how we hide empty private fields Consider this: ```rust mod foo { pub struct Bar { pub a: bool, b: !, } } fn match_a_bar(bar: foo::Bar) -> bool { match bar { Bar { a, .. } => a, } } ``` Because the field `b` is private, matches outside the module are not allowed to observe the fact that `Bar` is empty. In particular `match bar {}` is valid within the module `foo` but an error outside (assuming `exhaustive_patterns`). We currently handle this by hiding the field `b` when it's both private and empty. This means that the pattern `Bar { a, .. }` is lowered to `Bar(a, _)` if we're inside of `foo` and to `Bar(a)` outside. This involves a bit of a dance to keep field indices straight. But most importantly this makes pattern lowering depend on the module. In this PR, I instead do nothing special when lowering. Only during analysis do we track whether a place must be skipped. r? `@compiler-errors`
2 parents 384d26f + c918893 commit b5ae3af

File tree

5 files changed

+98
-88
lines changed

5 files changed

+98
-88
lines changed

‎compiler/rustc_pattern_analysis/src/constructor.rs

+6
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,9 @@ pub enum Constructor<Cx: TypeCx> {
688688
/// Fake extra constructor for constructors that are not seen in the matrix, as explained at the
689689
/// top of the file.
690690
Missing,
691+
/// Fake extra constructor that indicates and empty field that is private. When we encounter one
692+
/// we skip the column entirely so we don't observe its emptiness. Only used for specialization.
693+
PrivateUninhabited,
691694
}
692695

693696
impl<Cx: TypeCx> Clone for Constructor<Cx> {
@@ -709,6 +712,7 @@ impl<Cx: TypeCx> Clone for Constructor<Cx> {
709712
Constructor::NonExhaustive => Constructor::NonExhaustive,
710713
Constructor::Hidden => Constructor::Hidden,
711714
Constructor::Missing => Constructor::Missing,
715+
Constructor::PrivateUninhabited => Constructor::PrivateUninhabited,
712716
}
713717
}
714718
}
@@ -763,6 +767,8 @@ impl<Cx: TypeCx> Constructor<Cx> {
763767
}
764768
// Wildcards cover anything
765769
(_, Wildcard) => true,
770+
// `PrivateUninhabited` skips everything.
771+
(PrivateUninhabited, _) => true,
766772
// Only a wildcard pattern can match these special constructors.
767773
(Missing { .. } | NonExhaustive | Hidden, _) => false,
768774

‎compiler/rustc_pattern_analysis/src/lib.rs

+7-3
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ use crate::usefulness::{compute_match_usefulness, ValidityConstraint};
8282
pub trait Captures<'a> {}
8383
impl<'a, T: ?Sized> Captures<'a> for T {}
8484

85+
/// `bool` newtype that indicates whether this is a privately uninhabited field that we should skip
86+
/// during analysis.
87+
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
88+
pub struct PrivateUninhabitedField(pub bool);
89+
8590
/// Context that provides type information about constructors.
8691
///
8792
/// Most of the crate is parameterized on a type that implements this trait.
@@ -105,13 +110,12 @@ pub trait TypeCx: Sized + fmt::Debug {
105110
/// The number of fields for this constructor.
106111
fn ctor_arity(&self, ctor: &Constructor<Self>, ty: &Self::Ty) -> usize;
107112

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

116120
/// The set of all the constructors for `ty`.
117121
///

‎compiler/rustc_pattern_analysis/src/pat.rs

+11-8
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::fmt;
55
use smallvec::{smallvec, SmallVec};
66

77
use crate::constructor::{Constructor, Slice, SliceKind};
8-
use crate::TypeCx;
8+
use crate::{PrivateUninhabitedField, TypeCx};
99

1010
use self::Constructor::*;
1111

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

‎compiler/rustc_pattern_analysis/src/rustc.rs

+51-73
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,15 @@ use rustc_middle::mir::interpret::Scalar;
1010
use rustc_middle::mir::{self, Const};
1111
use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange, PatRangeBoundary};
1212
use rustc_middle::ty::layout::IntegerExt;
13-
use rustc_middle::ty::{self, OpaqueTypeKey, Ty, TyCtxt, TypeVisitableExt, VariantDef};
13+
use rustc_middle::ty::{self, FieldDef, OpaqueTypeKey, Ty, TyCtxt, TypeVisitableExt, VariantDef};
1414
use rustc_session::lint;
1515
use rustc_span::{ErrorGuaranteed, Span, DUMMY_SP};
1616
use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT};
1717

1818
use crate::constructor::{
1919
IntRange, MaybeInfiniteInt, OpaqueId, RangeEnd, Slice, SliceKind, VariantVisibility,
2020
};
21-
use crate::{errors, Captures, TypeCx};
21+
use crate::{errors, Captures, PrivateUninhabitedField, TypeCx};
2222

2323
use crate::constructor::Constructor::*;
2424

@@ -158,34 +158,19 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
158158
}
159159
}
160160

161-
// In the cases of either a `#[non_exhaustive]` field list or a non-public field, we hide
162-
// uninhabited fields in order not to reveal the uninhabitedness of the whole variant.
163-
// This lists the fields we keep along with their types.
164-
pub(crate) fn list_variant_nonhidden_fields(
161+
pub(crate) fn variant_sub_tys(
165162
&self,
166163
ty: RevealedTy<'tcx>,
167164
variant: &'tcx VariantDef,
168-
) -> impl Iterator<Item = (FieldIdx, RevealedTy<'tcx>)> + Captures<'p> + Captures<'_> {
169-
let cx = self;
170-
let ty::Adt(adt, args) = ty.kind() else { bug!() };
171-
// Whether we must not match the fields of this variant exhaustively.
172-
let is_non_exhaustive = variant.is_field_list_non_exhaustive() && !adt.did().is_local();
173-
174-
variant.fields.iter().enumerate().filter_map(move |(i, field)| {
175-
let ty = field.ty(cx.tcx, args);
165+
) -> impl Iterator<Item = (&'tcx FieldDef, RevealedTy<'tcx>)> + Captures<'p> + Captures<'_>
166+
{
167+
let ty::Adt(_, args) = ty.kind() else { bug!() };
168+
variant.fields.iter().map(move |field| {
169+
let ty = field.ty(self.tcx, args);
176170
// `field.ty()` doesn't normalize after instantiating.
177-
let ty = cx.tcx.normalize_erasing_regions(cx.param_env, ty);
178-
let is_visible = adt.is_enum() || field.vis.is_accessible_from(cx.module, cx.tcx);
179-
let is_uninhabited = (cx.tcx.features().exhaustive_patterns
180-
|| cx.tcx.features().min_exhaustive_patterns)
181-
&& cx.is_uninhabited(ty);
182-
183-
if is_uninhabited && (!is_visible || is_non_exhaustive) {
184-
None
185-
} else {
186-
let ty = cx.reveal_opaque_ty(ty);
187-
Some((FieldIdx::new(i), ty))
188-
}
171+
let ty = self.tcx.normalize_erasing_regions(self.param_env, ty);
172+
let ty = self.reveal_opaque_ty(ty);
173+
(field, ty)
189174
})
190175
}
191176

@@ -210,12 +195,17 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
210195
&'a self,
211196
ctor: &'a Constructor<'p, 'tcx>,
212197
ty: RevealedTy<'tcx>,
213-
) -> impl Iterator<Item = RevealedTy<'tcx>> + ExactSizeIterator + Captures<'a> {
198+
) -> impl Iterator<Item = (RevealedTy<'tcx>, PrivateUninhabitedField)>
199+
+ ExactSizeIterator
200+
+ Captures<'a> {
214201
fn reveal_and_alloc<'a, 'tcx>(
215202
cx: &'a RustcMatchCheckCtxt<'_, 'tcx>,
216203
iter: impl Iterator<Item = Ty<'tcx>>,
217-
) -> &'a [RevealedTy<'tcx>] {
218-
cx.dropless_arena.alloc_from_iter(iter.map(|ty| cx.reveal_opaque_ty(ty)))
204+
) -> &'a [(RevealedTy<'tcx>, PrivateUninhabitedField)] {
205+
cx.dropless_arena.alloc_from_iter(
206+
iter.map(|ty| cx.reveal_opaque_ty(ty))
207+
.map(|ty| (ty, PrivateUninhabitedField(false))),
208+
)
219209
}
220210
let cx = self;
221211
let slice = match ctor {
@@ -229,7 +219,21 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
229219
} else {
230220
let variant =
231221
&adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt));
232-
let tys = cx.list_variant_nonhidden_fields(ty, variant).map(|(_, ty)| ty);
222+
223+
// In the cases of either a `#[non_exhaustive]` field list or a non-public
224+
// field, we skip uninhabited fields in order not to reveal the
225+
// uninhabitedness of the whole variant.
226+
let is_non_exhaustive =
227+
variant.is_field_list_non_exhaustive() && !adt.did().is_local();
228+
let tys = cx.variant_sub_tys(ty, variant).map(|(field, ty)| {
229+
let is_visible =
230+
adt.is_enum() || field.vis.is_accessible_from(cx.module, cx.tcx);
231+
let is_uninhabited = (cx.tcx.features().exhaustive_patterns
232+
|| cx.tcx.features().min_exhaustive_patterns)
233+
&& cx.is_uninhabited(*ty);
234+
let skip = is_uninhabited && (!is_visible || is_non_exhaustive);
235+
(ty, PrivateUninhabitedField(skip))
236+
});
233237
cx.dropless_arena.alloc_from_iter(tys)
234238
}
235239
}
@@ -246,16 +250,8 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
246250
}
247251
_ => bug!("bad slice pattern {:?} {:?}", ctor, ty),
248252
},
249-
Bool(..)
250-
| IntRange(..)
251-
| F32Range(..)
252-
| F64Range(..)
253-
| Str(..)
254-
| Opaque(..)
255-
| NonExhaustive
256-
| Hidden
257-
| Missing { .. }
258-
| Wildcard => &[],
253+
Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..)
254+
| NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => &[],
259255
Or => {
260256
bug!("called `Fields::wildcards` on an `Or` ctor")
261257
}
@@ -274,25 +270,16 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
274270
// patterns. If we're here we can assume this is a box pattern.
275271
1
276272
} else {
277-
let variant =
278-
&adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt));
279-
self.list_variant_nonhidden_fields(ty, variant).count()
273+
let variant_idx = RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt);
274+
adt.variant(variant_idx).fields.len()
280275
}
281276
}
282277
_ => bug!("Unexpected type for constructor `{ctor:?}`: {ty:?}"),
283278
},
284279
Ref => 1,
285280
Slice(slice) => slice.arity(),
286-
Bool(..)
287-
| IntRange(..)
288-
| F32Range(..)
289-
| F64Range(..)
290-
| Str(..)
291-
| Opaque(..)
292-
| NonExhaustive
293-
| Hidden
294-
| Missing { .. }
295-
| Wildcard => 0,
281+
Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..)
282+
| NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => 0,
296283
Or => bug!("The `Or` constructor doesn't have a fixed arity"),
297284
}
298285
}
@@ -520,20 +507,12 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
520507
};
521508
let variant =
522509
&adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt));
523-
// For each field in the variant, we store the relevant index into `self.fields` if any.
524-
let mut field_id_to_id: Vec<Option<usize>> =
525-
(0..variant.fields.len()).map(|_| None).collect();
526-
let tys = cx.list_variant_nonhidden_fields(ty, variant).enumerate().map(
527-
|(i, (field, ty))| {
528-
field_id_to_id[field.index()] = Some(i);
529-
ty
530-
},
531-
);
532-
fields = tys.map(|ty| DeconstructedPat::wildcard(ty)).collect();
510+
fields = cx
511+
.variant_sub_tys(ty, variant)
512+
.map(|(_, ty)| DeconstructedPat::wildcard(ty))
513+
.collect();
533514
for pat in subpatterns {
534-
if let Some(i) = field_id_to_id[pat.field.index()] {
535-
fields[i] = self.lower_pat(&pat.pattern);
536-
}
515+
fields[pat.field.index()] = self.lower_pat(&pat.pattern);
537516
}
538517
}
539518
_ => bug!("pattern has unexpected type: pat: {:?}, ty: {:?}", pat, ty),
@@ -775,11 +754,9 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
775754
ty::Adt(adt_def, args) => {
776755
let variant_index =
777756
RustcMatchCheckCtxt::variant_index_for_adt(&pat.ctor(), *adt_def);
778-
let variant = &adt_def.variant(variant_index);
779-
let subpatterns = cx
780-
.list_variant_nonhidden_fields(*pat.ty(), variant)
781-
.zip(subpatterns)
782-
.map(|((field, _ty), pattern)| FieldPat { field, pattern })
757+
let subpatterns = subpatterns
758+
.enumerate()
759+
.map(|(i, pattern)| FieldPat { field: FieldIdx::new(i), pattern })
783760
.collect();
784761

785762
if adt_def.is_enum() {
@@ -830,7 +807,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> {
830807
}
831808
}
832809
&Str(value) => PatKind::Constant { value },
833-
Wildcard | NonExhaustive | Hidden => PatKind::Wild,
810+
Wildcard | NonExhaustive | Hidden | PrivateUninhabited => PatKind::Wild,
834811
Missing { .. } => bug!(
835812
"trying to convert a `Missing` constructor into a `Pat`; this is probably a bug,
836813
`Missing` should have been processed in `apply_constructors`"
@@ -866,7 +843,8 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> {
866843
&'a self,
867844
ctor: &'a crate::constructor::Constructor<Self>,
868845
ty: &'a Self::Ty,
869-
) -> impl Iterator<Item = Self::Ty> + ExactSizeIterator + Captures<'a> {
846+
) -> impl Iterator<Item = (Self::Ty, PrivateUninhabitedField)> + ExactSizeIterator + Captures<'a>
847+
{
870848
self.ctor_sub_tys(ctor, *ty)
871849
}
872850
fn ctors_for_ty(

‎compiler/rustc_pattern_analysis/src/usefulness.rs

+23-4
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,7 @@ use std::fmt;
716716

717717
use crate::constructor::{Constructor, ConstructorSet, IntRange};
718718
use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat};
719-
use crate::{Captures, MatchArm, TypeCx};
719+
use crate::{Captures, MatchArm, PrivateUninhabitedField, TypeCx};
720720

721721
use self::ValidityConstraint::*;
722722

@@ -817,6 +817,9 @@ impl fmt::Display for ValidityConstraint {
817817
struct PlaceInfo<Cx: TypeCx> {
818818
/// The type of the place.
819819
ty: Cx::Ty,
820+
/// Whether the place is a private uninhabited field. If so we skip this field during analysis
821+
/// so that we don't observe its emptiness.
822+
private_uninhabited: bool,
820823
/// Whether the place is known to contain valid data.
821824
validity: ValidityConstraint,
822825
/// Whether the place is the scrutinee itself or a subplace of it.
@@ -833,8 +836,9 @@ impl<Cx: TypeCx> PlaceInfo<Cx> {
833836
) -> impl Iterator<Item = Self> + ExactSizeIterator + Captures<'a> {
834837
let ctor_sub_tys = cx.ctor_sub_tys(ctor, &self.ty);
835838
let ctor_sub_validity = self.validity.specialize(ctor);
836-
ctor_sub_tys.map(move |ty| PlaceInfo {
839+
ctor_sub_tys.map(move |(ty, PrivateUninhabitedField(private_uninhabited))| PlaceInfo {
837840
ty,
841+
private_uninhabited,
838842
validity: ctor_sub_validity,
839843
is_scrutinee: false,
840844
})
@@ -856,6 +860,11 @@ impl<Cx: TypeCx> PlaceInfo<Cx> {
856860
where
857861
Cx: 'a,
858862
{
863+
if self.private_uninhabited {
864+
// Skip the whole column
865+
return Ok((smallvec![Constructor::PrivateUninhabited], vec![]));
866+
}
867+
859868
let ctors_for_ty = cx.ctors_for_ty(&self.ty)?;
860869

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

915924
impl<Cx: TypeCx> Clone for PlaceInfo<Cx> {
916925
fn clone(&self) -> Self {
917-
Self { ty: self.ty.clone(), validity: self.validity, is_scrutinee: self.is_scrutinee }
926+
Self {
927+
ty: self.ty.clone(),
928+
private_uninhabited: self.private_uninhabited,
929+
validity: self.validity,
930+
is_scrutinee: self.is_scrutinee,
931+
}
918932
}
919933
}
920934

@@ -1121,7 +1135,12 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
11211135
scrut_ty: Cx::Ty,
11221136
scrut_validity: ValidityConstraint,
11231137
) -> Self {
1124-
let place_info = PlaceInfo { ty: scrut_ty, validity: scrut_validity, is_scrutinee: true };
1138+
let place_info = PlaceInfo {
1139+
ty: scrut_ty,
1140+
private_uninhabited: false,
1141+
validity: scrut_validity,
1142+
is_scrutinee: true,
1143+
};
11251144
let mut matrix = Matrix {
11261145
rows: Vec::with_capacity(arms.len()),
11271146
place_info: smallvec![place_info],

0 commit comments

Comments
 (0)