Rollup merge of #126932 - Zalathar:flat-pat, r=Nadrieril

Tweak `FlatPat::new` to avoid a temporarily-invalid state

It was somewhat confusing that the old constructor would create a `FlatPat` in a (possibly) non-simplified state, and then simplify its contents in-place.

So instead we now create its fields as local variables, perform simplification, and then create the struct afterwards.

This doesn't affect correctness, but is less confusing.

---

I've also included some semi-related comments that I made while trying to navigate this code.
This commit is contained in:
Matthias Krüger 2024-06-25 21:33:44 +02:00 committed by GitHub
commit 7e1489cf63
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 24 additions and 11 deletions

View File

@ -1034,6 +1034,12 @@ impl<'tcx> PatternExtraData<'tcx> {
} }
/// A pattern in a form suitable for generating code. /// A pattern in a form suitable for generating code.
///
/// Here, "flat" indicates that the pattern's match pairs have been recursively
/// simplified by [`Builder::simplify_match_pairs`]. They are not necessarily
/// flat in an absolute sense.
///
/// Will typically be incorporated into a [`Candidate`].
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
struct FlatPat<'pat, 'tcx> { struct FlatPat<'pat, 'tcx> {
/// To match the pattern, all of these must be satisfied... /// To match the pattern, all of these must be satisfied...
@ -1045,23 +1051,25 @@ struct FlatPat<'pat, 'tcx> {
} }
impl<'tcx, 'pat> FlatPat<'pat, 'tcx> { impl<'tcx, 'pat> FlatPat<'pat, 'tcx> {
/// Creates a `FlatPat` containing a simplified [`MatchPair`] list/forest
/// for the given pattern.
fn new( fn new(
place: PlaceBuilder<'tcx>, place: PlaceBuilder<'tcx>,
pattern: &'pat Pat<'tcx>, pattern: &'pat Pat<'tcx>,
cx: &mut Builder<'_, 'tcx>, cx: &mut Builder<'_, 'tcx>,
) -> Self { ) -> Self {
let is_never = pattern.is_never_pattern(); // First, recursively build a tree of match pairs for the given pattern.
let mut flat_pat = FlatPat { let mut match_pairs = vec![MatchPair::new(place, pattern, cx)];
match_pairs: vec![MatchPair::new(place, pattern, cx)], let mut extra_data = PatternExtraData {
extra_data: PatternExtraData { span: pattern.span,
span: pattern.span, bindings: Vec::new(),
bindings: Vec::new(), ascriptions: Vec::new(),
ascriptions: Vec::new(), is_never: pattern.is_never_pattern(),
is_never,
},
}; };
cx.simplify_match_pairs(&mut flat_pat.match_pairs, &mut flat_pat.extra_data); // Partly-flatten and sort the match pairs, while recording extra data.
flat_pat cx.simplify_match_pairs(&mut match_pairs, &mut extra_data);
Self { match_pairs, extra_data }
} }
} }
@ -1107,9 +1115,12 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> {
has_guard: bool, has_guard: bool,
cx: &mut Builder<'_, 'tcx>, cx: &mut Builder<'_, 'tcx>,
) -> Self { ) -> Self {
// Use `FlatPat` to build simplified match pairs, then immediately
// incorporate them into a new candidate.
Self::from_flat_pat(FlatPat::new(place, pattern, cx), has_guard) Self::from_flat_pat(FlatPat::new(place, pattern, cx), has_guard)
} }
/// Incorporates an already-simplified [`FlatPat`] into a new candidate.
fn from_flat_pat(flat_pat: FlatPat<'pat, 'tcx>, has_guard: bool) -> Self { fn from_flat_pat(flat_pat: FlatPat<'pat, 'tcx>, has_guard: bool) -> Self {
Candidate { Candidate {
match_pairs: flat_pat.match_pairs, match_pairs: flat_pat.match_pairs,

View File

@ -95,6 +95,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
} }
impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
/// Recursively builds a `MatchPair` tree for the given pattern and its
/// subpatterns.
pub(in crate::build) fn new( pub(in crate::build) fn new(
mut place_builder: PlaceBuilder<'tcx>, mut place_builder: PlaceBuilder<'tcx>,
pattern: &'pat Pat<'tcx>, pattern: &'pat Pat<'tcx>,