Skip to content

Commit c27d914

Browse files
committed
Auto merge of #119688 - Nadrieril:dont-alloc-custom-wildcards, r=<try>
Exhaustiveness: use an `Option` instead of allocating fictitious patterns In the process of exhaustiveness checking, `Matrix` stores a 2D array of patterns. Those are subpatterns of the patterns we were provided as input, _except_ sometimes we allocate some extra wildcard patterns to fill a hole during specialization. Morally though, we could store `Option<&'p DeconstructedPat>` in the matrix, where `None` signifies a wildcard. That way we'd only have "real" patterns in the matrix and we wouldn't need the arena to allocate these wildcards. This is what this PR does. This is part of me splitting up #119581 for ease of review. r? `@compiler-errors`
2 parents 75c68cf + 1a3edc1 commit c27d914

File tree

3 files changed

+115
-68
lines changed

3 files changed

+115
-68
lines changed

compiler/rustc_pattern_analysis/src/lints.rs

+18-24
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use crate::errors::{
1111
NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Overlap,
1212
OverlappingRangeEndpoints, Uncovered,
1313
};
14+
use crate::pat::PatOrWild;
1415
use crate::rustc::{
1516
Constructor, DeconstructedPat, MatchArm, MatchCtxt, PlaceCtxt, RevealedTy, RustcMatchCheckCtxt,
1617
SplitConstructorSet, WitnessPat,
@@ -33,20 +34,24 @@ pub(crate) struct PatternColumn<'p, 'tcx> {
3334

3435
impl<'p, 'tcx> PatternColumn<'p, 'tcx> {
3536
pub(crate) fn new(arms: &[MatchArm<'p, 'tcx>]) -> Self {
36-
let mut patterns = Vec::with_capacity(arms.len());
37+
let patterns = Vec::with_capacity(arms.len());
38+
let mut column = PatternColumn { patterns };
3739
for arm in arms {
38-
if arm.pat.is_or_pat() {
39-
patterns.extend(arm.pat.flatten_or_pat())
40-
} else {
41-
patterns.push(arm.pat)
42-
}
40+
column.expand_and_push(PatOrWild::Pat(arm.pat));
4341
}
44-
Self { patterns }
42+
column
4543
}
46-
47-
fn is_empty(&self) -> bool {
48-
self.patterns.is_empty()
44+
fn expand_and_push(&mut self, pat: PatOrWild<'p, RustcMatchCheckCtxt<'p, 'tcx>>) {
45+
// We flatten or-patterns and skip wildcards
46+
if pat.is_or_pat() {
47+
self.patterns.extend(
48+
pat.flatten_or_pat().into_iter().filter_map(|pat_or_wild| pat_or_wild.as_pat()),
49+
)
50+
} else if let Some(pat) = pat.as_pat() {
51+
self.patterns.push(pat)
52+
}
4953
}
54+
5055
fn head_ty(&self) -> Option<RevealedTy<'tcx>> {
5156
self.patterns.first().map(|pat| pat.ty())
5257
}
@@ -83,23 +88,12 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> {
8388
(0..arity).map(|_| Self { patterns: Vec::new() }).collect();
8489
let relevant_patterns =
8590
self.patterns.iter().filter(|pat| ctor.is_covered_by(pcx, pat.ctor()));
86-
let ctor_sub_tys = pcx.ctor_sub_tys(ctor);
8791
for pat in relevant_patterns {
88-
let specialized = pat.specialize(pcx, ctor, ctor_sub_tys);
89-
for (subpat, column) in specialized.iter().zip(&mut specialized_columns) {
90-
if subpat.is_or_pat() {
91-
column.patterns.extend(subpat.flatten_or_pat())
92-
} else {
93-
column.patterns.push(subpat)
94-
}
92+
let specialized = pat.specialize(ctor, arity);
93+
for (subpat, column) in specialized.into_iter().zip(&mut specialized_columns) {
94+
column.expand_and_push(subpat);
9595
}
9696
}
97-
98-
assert!(
99-
!specialized_columns[0].is_empty(),
100-
"ctor {ctor:?} was listed as present but isn't;
101-
there is an inconsistency between `Constructor::is_covered_by` and `ConstructorSet::split`"
102-
);
10397
specialized_columns
10498
}
10599
}

compiler/rustc_pattern_analysis/src/pat.rs

+81-23
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::usefulness::PlaceCtxt;
1010
use crate::{Captures, TypeCx};
1111

1212
use self::Constructor::*;
13+
use self::PatOrWild::*;
1314

1415
/// Values and patterns can be represented as a constructor applied to some fields. This represents
1516
/// a pattern in this form.
@@ -50,14 +51,6 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> {
5051
pub(crate) fn is_or_pat(&self) -> bool {
5152
matches!(self.ctor, Or)
5253
}
53-
/// Expand this (possibly-nested) or-pattern into its alternatives.
54-
pub(crate) fn flatten_or_pat(&self) -> SmallVec<[&Self; 1]> {
55-
if self.is_or_pat() {
56-
self.iter_fields().flat_map(|p| p.flatten_or_pat()).collect()
57-
} else {
58-
smallvec![self]
59-
}
60-
}
6154

6255
pub fn ctor(&self) -> &Constructor<Cx> {
6356
&self.ctor
@@ -79,17 +72,10 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> {
7972
/// `other_ctor` can be different from `self.ctor`, but must be covered by it.
8073
pub(crate) fn specialize(
8174
&self,
82-
pcx: &PlaceCtxt<'_, 'p, Cx>,
8375
other_ctor: &Constructor<Cx>,
84-
ctor_sub_tys: &[Cx::Ty],
85-
) -> SmallVec<[&'p DeconstructedPat<'p, Cx>; 2]> {
86-
let wildcard_sub_tys = || {
87-
ctor_sub_tys
88-
.iter()
89-
.map(|ty| DeconstructedPat::wildcard(*ty))
90-
.map(|pat| pcx.mcx.wildcard_arena.alloc(pat) as &_)
91-
.collect()
92-
};
76+
ctor_arity: usize,
77+
) -> SmallVec<[PatOrWild<'p, Cx>; 2]> {
78+
let wildcard_sub_tys = || (0..ctor_arity).map(|_| Wild).collect();
9379
match (&self.ctor, other_ctor) {
9480
// Return a wildcard for each field of `other_ctor`.
9581
(Wildcard, _) => wildcard_sub_tys(),
@@ -105,14 +91,14 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> {
10591
// Fill in the fields from both ends.
10692
let new_arity = fields.len();
10793
for i in 0..prefix {
108-
fields[i] = &self.fields[i];
94+
fields[i] = Pat(&self.fields[i]);
10995
}
11096
for i in 0..suffix {
111-
fields[new_arity - 1 - i] = &self.fields[self.fields.len() - 1 - i];
97+
fields[new_arity - 1 - i] = Pat(&self.fields[self.fields.len() - 1 - i]);
11298
}
11399
fields
114100
}
115-
_ => self.fields.iter().collect(),
101+
_ => self.fields.iter().map(Pat).collect(),
116102
}
117103
}
118104

@@ -153,14 +139,86 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> {
153139
}
154140
}
155141

156-
/// This is mostly copied from the `Pat` impl. This is best effort and not good enough for a
157-
/// `Display` impl.
142+
/// This is best effort and not good enough for a `Display` impl.
158143
impl<'p, Cx: TypeCx> fmt::Debug for DeconstructedPat<'p, Cx> {
159144
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
160145
Cx::debug_pat(f, self)
161146
}
162147
}
163148

149+
/// Represents either a pattern obtained from user input or a wildcard constructed during the
150+
/// algorithm. Do not use `Wild` to represent a wildcard pattern comping from user input.
151+
///
152+
/// This is morally `Option<&'p DeconstructedPat>` where `None` is interpreted as a wildcard.
153+
#[derive(derivative::Derivative)]
154+
#[derivative(Clone(bound = ""), Copy(bound = ""))]
155+
pub(crate) enum PatOrWild<'p, Cx: TypeCx> {
156+
/// A non-user-provided wildcard, created during specialization.
157+
Wild,
158+
/// A user-provided pattern.
159+
Pat(&'p DeconstructedPat<'p, Cx>),
160+
}
161+
162+
impl<'p, Cx: TypeCx> PatOrWild<'p, Cx> {
163+
pub(crate) fn as_pat(&self) -> Option<&'p DeconstructedPat<'p, Cx>> {
164+
match self {
165+
Wild => None,
166+
Pat(pat) => Some(pat),
167+
}
168+
}
169+
pub(crate) fn ctor(self) -> &'p Constructor<Cx> {
170+
match self {
171+
Wild => &Wildcard,
172+
Pat(pat) => pat.ctor(),
173+
}
174+
}
175+
176+
pub(crate) fn is_or_pat(&self) -> bool {
177+
match self {
178+
Wild => false,
179+
Pat(pat) => pat.is_or_pat(),
180+
}
181+
}
182+
183+
/// Expand this (possibly-nested) or-pattern into its alternatives.
184+
pub(crate) fn flatten_or_pat(self) -> SmallVec<[Self; 1]> {
185+
match self {
186+
Pat(pat) if pat.is_or_pat() => {
187+
pat.iter_fields().flat_map(|p| Pat(p).flatten_or_pat()).collect()
188+
}
189+
_ => smallvec![self],
190+
}
191+
}
192+
193+
/// Specialize this pattern with a constructor.
194+
/// `other_ctor` can be different from `self.ctor`, but must be covered by it.
195+
pub(crate) fn specialize(
196+
&self,
197+
other_ctor: &Constructor<Cx>,
198+
ctor_arity: usize,
199+
) -> SmallVec<[PatOrWild<'p, Cx>; 2]> {
200+
match self {
201+
Wild => (0..ctor_arity).map(|_| Wild).collect(),
202+
Pat(pat) => pat.specialize(other_ctor, ctor_arity),
203+
}
204+
}
205+
206+
pub(crate) fn set_useful(&self) {
207+
if let Pat(pat) = self {
208+
pat.set_useful()
209+
}
210+
}
211+
}
212+
213+
impl<'p, Cx: TypeCx> fmt::Debug for PatOrWild<'p, Cx> {
214+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
215+
match self {
216+
Wild => write!(f, "_"),
217+
Pat(pat) => pat.fmt(f),
218+
}
219+
}
220+
}
221+
164222
/// Same idea as `DeconstructedPat`, except this is a fictitious pattern built up for diagnostics
165223
/// purposes. As such they don't use interning and can be cloned.
166224
#[derive(derivative::Derivative)]

compiler/rustc_pattern_analysis/src/usefulness.rs

+16-21
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,7 @@ use smallvec::{smallvec, SmallVec};
716716
use std::fmt;
717717

718718
use crate::constructor::{Constructor, ConstructorSet};
719-
use crate::pat::{DeconstructedPat, WitnessPat};
719+
use crate::pat::{DeconstructedPat, PatOrWild, WitnessPat};
720720
use crate::{Captures, MatchArm, MatchCtxt, TypeCx};
721721

722722
use self::ValidityConstraint::*;
@@ -827,7 +827,7 @@ impl fmt::Display for ValidityConstraint {
827827
#[derivative(Clone(bound = ""))]
828828
struct PatStack<'p, Cx: TypeCx> {
829829
// Rows of len 1 are very common, which is why `SmallVec[_; 2]` works well.
830-
pats: SmallVec<[&'p DeconstructedPat<'p, Cx>; 2]>,
830+
pats: SmallVec<[PatOrWild<'p, Cx>; 2]>,
831831
/// Sometimes we know that as far as this row is concerned, the current case is already handled
832832
/// by a different, more general, case. When the case is irrelevant for all rows this allows us
833833
/// to skip a case entirely. This is purely an optimization. See at the top for details.
@@ -836,7 +836,7 @@ struct PatStack<'p, Cx: TypeCx> {
836836

837837
impl<'p, Cx: TypeCx> PatStack<'p, Cx> {
838838
fn from_pattern(pat: &'p DeconstructedPat<'p, Cx>) -> Self {
839-
PatStack { pats: smallvec![pat], relevant: true }
839+
PatStack { pats: smallvec![PatOrWild::Pat(pat)], relevant: true }
840840
}
841841

842842
fn is_empty(&self) -> bool {
@@ -847,14 +847,11 @@ impl<'p, Cx: TypeCx> PatStack<'p, Cx> {
847847
self.pats.len()
848848
}
849849

850-
fn head_opt(&self) -> Option<&'p DeconstructedPat<'p, Cx>> {
851-
self.pats.first().copied()
852-
}
853-
fn head(&self) -> &'p DeconstructedPat<'p, Cx> {
854-
self.head_opt().unwrap()
850+
fn head(&self) -> PatOrWild<'p, Cx> {
851+
self.pats[0]
855852
}
856853

857-
fn iter(&self) -> impl Iterator<Item = &'p DeconstructedPat<'p, Cx>> + Captures<'_> {
854+
fn iter(&self) -> impl Iterator<Item = PatOrWild<'p, Cx>> + Captures<'_> {
858855
self.pats.iter().copied()
859856
}
860857

@@ -872,14 +869,13 @@ impl<'p, Cx: TypeCx> PatStack<'p, Cx> {
872869
/// Only call if `ctor.is_covered_by(self.head().ctor())` is true.
873870
fn pop_head_constructor(
874871
&self,
875-
pcx: &PlaceCtxt<'_, 'p, Cx>,
876872
ctor: &Constructor<Cx>,
877-
ctor_sub_tys: &[Cx::Ty],
873+
ctor_arity: usize,
878874
ctor_is_relevant: bool,
879875
) -> PatStack<'p, Cx> {
880876
// We pop the head pattern and push the new fields extracted from the arguments of
881877
// `self.head()`.
882-
let mut new_pats = self.head().specialize(pcx, ctor, ctor_sub_tys);
878+
let mut new_pats = self.head().specialize(ctor, ctor_arity);
883879
new_pats.extend_from_slice(&self.pats[1..]);
884880
// `ctor` is relevant for this row if it is the actual constructor of this row, or if the
885881
// row has a wildcard and `ctor` is relevant for wildcards.
@@ -926,11 +922,11 @@ impl<'p, Cx: TypeCx> MatrixRow<'p, Cx> {
926922
self.pats.len()
927923
}
928924

929-
fn head(&self) -> &'p DeconstructedPat<'p, Cx> {
925+
fn head(&self) -> PatOrWild<'p, Cx> {
930926
self.pats.head()
931927
}
932928

933-
fn iter(&self) -> impl Iterator<Item = &'p DeconstructedPat<'p, Cx>> + Captures<'_> {
929+
fn iter(&self) -> impl Iterator<Item = PatOrWild<'p, Cx>> + Captures<'_> {
934930
self.pats.iter()
935931
}
936932

@@ -949,14 +945,13 @@ impl<'p, Cx: TypeCx> MatrixRow<'p, Cx> {
949945
/// Only call if `ctor.is_covered_by(self.head().ctor())` is true.
950946
fn pop_head_constructor(
951947
&self,
952-
pcx: &PlaceCtxt<'_, 'p, Cx>,
953948
ctor: &Constructor<Cx>,
954-
ctor_sub_tys: &[Cx::Ty],
949+
ctor_arity: usize,
955950
ctor_is_relevant: bool,
956951
parent_row: usize,
957952
) -> MatrixRow<'p, Cx> {
958953
MatrixRow {
959-
pats: self.pats.pop_head_constructor(pcx, ctor, ctor_sub_tys, ctor_is_relevant),
954+
pats: self.pats.pop_head_constructor(ctor, ctor_arity, ctor_is_relevant),
960955
parent_row,
961956
is_under_guard: self.is_under_guard,
962957
useful: false,
@@ -1054,7 +1049,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
10541049
}
10551050

10561051
/// Iterate over the first pattern of each row.
1057-
fn heads(&self) -> impl Iterator<Item = &'p DeconstructedPat<'p, Cx>> + Clone + Captures<'_> {
1052+
fn heads(&self) -> impl Iterator<Item = PatOrWild<'p, Cx>> + Clone + Captures<'_> {
10581053
self.rows().map(|r| r.head())
10591054
}
10601055

@@ -1066,11 +1061,12 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
10661061
ctor_is_relevant: bool,
10671062
) -> Matrix<'p, Cx> {
10681063
let ctor_sub_tys = pcx.ctor_sub_tys(ctor);
1064+
let arity = ctor_sub_tys.len();
10691065
let specialized_place_ty =
10701066
ctor_sub_tys.iter().chain(self.place_ty[1..].iter()).copied().collect();
10711067
let ctor_sub_validity = self.place_validity[0].specialize(ctor);
10721068
let specialized_place_validity = std::iter::repeat(ctor_sub_validity)
1073-
.take(ctor.arity(pcx))
1069+
.take(arity)
10741070
.chain(self.place_validity[1..].iter().copied())
10751071
.collect();
10761072
let mut matrix = Matrix {
@@ -1081,8 +1077,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
10811077
};
10821078
for (i, row) in self.rows().enumerate() {
10831079
if ctor.is_covered_by(pcx, row.head().ctor()) {
1084-
let new_row =
1085-
row.pop_head_constructor(pcx, ctor, ctor_sub_tys, ctor_is_relevant, i);
1080+
let new_row = row.pop_head_constructor(ctor, arity, ctor_is_relevant, i);
10861081
matrix.expand_and_push(new_row);
10871082
}
10881083
}

0 commit comments

Comments
 (0)