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

Zalathar · Zalathar · commit c2f1072e013c · 2024-06-25T17:46:35.000+10:00
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.
diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs
@@ -1031,6 +1031,12 @@ impl<'tcx> PatternExtraData<'tcx> {
 }
 
 /// 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)]
 struct FlatPat<'pat, 'tcx> {
     /// To match the pattern, all of these must be satisfied...
@@ -1042,23 +1048,25 @@ struct FlatPat<'pat, 'tcx> {
 }
 
 impl<'tcx, 'pat> FlatPat<'pat, 'tcx> {
+    /// Creates a `FlatPat` containing a simplified [`MatchPair`] list/forest
+    /// for the given pattern.
     fn new(
         place: PlaceBuilder<'tcx>,
         pattern: &'pat Pat<'tcx>,
         cx: &mut Builder<'_, 'tcx>,
     ) -> Self {
-        let is_never = pattern.is_never_pattern();
-        let mut flat_pat = FlatPat {
-            match_pairs: vec![MatchPair::new(place, pattern, cx)],
-            extra_data: PatternExtraData {
-                span: pattern.span,
-                bindings: Vec::new(),
-                ascriptions: Vec::new(),
-                is_never,
-            },
+        // First, recursively build a tree of match pairs for the given pattern.
+        let mut match_pairs = vec![MatchPair::new(place, pattern, cx)];
+        let mut extra_data = PatternExtraData {
+            span: pattern.span,
+            bindings: Vec::new(),
+            ascriptions: Vec::new(),
+            is_never: pattern.is_never_pattern(),
         };
-        cx.simplify_match_pairs(&mut flat_pat.match_pairs, &mut flat_pat.extra_data);
-        flat_pat
+        // Partly-flatten and sort the match pairs, while recording extra data.
+        cx.simplify_match_pairs(&mut match_pairs, &mut extra_data);
+
+        Self { match_pairs, extra_data }
     }
 }
 
@@ -1104,9 +1112,12 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> {
         has_guard: bool,
         cx: &mut Builder<'_, 'tcx>,
     ) -> 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)
     }
 
+    /// Incorporates an already-simplified [`FlatPat`] into a new candidate.
     fn from_flat_pat(flat_pat: FlatPat<'pat, 'tcx>, has_guard: bool) -> Self {
         Candidate {
             match_pairs: flat_pat.match_pairs,
diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs
@@ -95,6 +95,8 @@ impl<'a, 'tcx> Builder<'a, '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(
         mut place_builder: PlaceBuilder<'tcx>,
         pattern: &'pat Pat<'tcx>,

Original file line number	Diff line number	Diff line change
`@@ -95,6 +95,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {`
`95`	`95`	`}`
`96`	`96`
`97`	`97`	`impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {`
	`98`	+ /// Recursively builds a `MatchPair` tree for the given pattern and its
	`99`	`+ /// subpatterns.`
`98`	`100`	`pub(in crate::build) fn new(`
`99`	`101`	`mut place_builder: PlaceBuilder<'tcx>,`
`100`	`102`	`pattern: &'pat Pat<'tcx>,`