From 7a31456c1ce6e9bbf878faf225422bd31ad09e9b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 5 Mar 2025 23:25:59 +1100 Subject: [PATCH 1/6] Make `field_match_pairs` push its output nodes to a vector --- .../src/builder/matches/match_pair.rs | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs index 10b43390eb288..cdac35ea0c749 100644 --- a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs +++ b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs @@ -9,24 +9,21 @@ use crate::builder::expr::as_place::{PlaceBase, PlaceBuilder}; use crate::builder::matches::{FlatPat, MatchPairTree, TestCase}; impl<'a, 'tcx> Builder<'a, 'tcx> { - /// Builds and returns [`MatchPairTree`] subtrees, one for each pattern in + /// Builds and pushes [`MatchPairTree`] subtrees, one for each pattern in /// `subpatterns`, representing the fields of a [`PatKind::Variant`] or /// [`PatKind::Leaf`]. /// /// Used internally by [`MatchPairTree::for_pattern`]. fn field_match_pairs( &mut self, + match_pairs: &mut Vec>, place: PlaceBuilder<'tcx>, subpatterns: &[FieldPat<'tcx>], - ) -> Vec> { - subpatterns - .iter() - .map(|fieldpat| { - let place = - place.clone_project(PlaceElem::Field(fieldpat.field, fieldpat.pattern.ty)); - MatchPairTree::for_pattern(place, &fieldpat.pattern, self) - }) - .collect() + ) { + for fieldpat in subpatterns { + let place = place.clone_project(PlaceElem::Field(fieldpat.field, fieldpat.pattern.ty)); + match_pairs.push(MatchPairTree::for_pattern(place, &fieldpat.pattern, self)); + } } /// Builds [`MatchPairTree`] subtrees for the prefix/middle/suffix parts of an @@ -215,7 +212,7 @@ impl<'tcx> MatchPairTree<'tcx> { PatKind::Variant { adt_def, variant_index, args, ref subpatterns } => { let downcast_place = place_builder.downcast(adt_def, variant_index); // `(x as Variant)` - subpairs = cx.field_match_pairs(downcast_place, subpatterns); + cx.field_match_pairs(&mut subpairs, downcast_place, subpatterns); let irrefutable = adt_def.variants().iter_enumerated().all(|(i, v)| { i == variant_index @@ -233,7 +230,7 @@ impl<'tcx> MatchPairTree<'tcx> { } PatKind::Leaf { ref subpatterns } => { - subpairs = cx.field_match_pairs(place_builder, subpatterns); + cx.field_match_pairs(&mut subpairs, place_builder, subpatterns); default_irrefutable() } From 281455add74530e23af760d60f6665a6901afc8b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 5 Mar 2025 23:25:59 +1100 Subject: [PATCH 2/6] Make `MatchPairTree::for_pattern` push its output node to a vector --- .../src/builder/matches/match_pair.rs | 39 ++++++++++--------- .../src/builder/matches/mod.rs | 3 +- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs index cdac35ea0c749..7252dadc3202a 100644 --- a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs +++ b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs @@ -22,7 +22,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ) { for fieldpat in subpatterns { let place = place.clone_project(PlaceElem::Field(fieldpat.field, fieldpat.pattern.ty)); - match_pairs.push(MatchPairTree::for_pattern(place, &fieldpat.pattern, self)); + MatchPairTree::for_pattern(place, &fieldpat.pattern, self, match_pairs); } } @@ -53,11 +53,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ((prefix.len() + suffix.len()).try_into().unwrap(), false) }; - match_pairs.extend(prefix.iter().enumerate().map(|(idx, subpattern)| { + for (idx, subpattern) in prefix.iter().enumerate() { let elem = ProjectionElem::ConstantIndex { offset: idx as u64, min_length, from_end: false }; - MatchPairTree::for_pattern(place.clone_project(elem), subpattern, self) - })); + let place = place.clone_project(elem); + MatchPairTree::for_pattern(place, subpattern, self, match_pairs); + } if let Some(subslice_pat) = opt_slice { let suffix_len = suffix.len() as u64; @@ -66,10 +67,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { to: if exact_size { min_length - suffix_len } else { suffix_len }, from_end: !exact_size, }); - match_pairs.push(MatchPairTree::for_pattern(subslice, subslice_pat, self)); + MatchPairTree::for_pattern(subslice, subslice_pat, self, match_pairs); } - match_pairs.extend(suffix.iter().rev().enumerate().map(|(idx, subpattern)| { + for (idx, subpattern) in suffix.iter().rev().enumerate() { let end_offset = (idx + 1) as u64; let elem = ProjectionElem::ConstantIndex { offset: if exact_size { min_length - end_offset } else { end_offset }, @@ -77,8 +78,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { from_end: !exact_size, }; let place = place.clone_project(elem); - MatchPairTree::for_pattern(place, subpattern, self) - })); + MatchPairTree::for_pattern(place, subpattern, self, match_pairs); + } } } @@ -89,7 +90,8 @@ impl<'tcx> MatchPairTree<'tcx> { mut place_builder: PlaceBuilder<'tcx>, pattern: &Pat<'tcx>, cx: &mut Builder<'_, 'tcx>, - ) -> MatchPairTree<'tcx> { + match_pairs: &mut Vec, // Newly-created nodes are added to this vector + ) { // Force the place type to the pattern's type. // FIXME(oli-obk): can we use this to simplify slice/array pattern hacks? if let Some(resolved) = place_builder.resolve_upvar(cx) { @@ -142,7 +144,7 @@ impl<'tcx> MatchPairTree<'tcx> { variance, }); - subpairs.push(MatchPairTree::for_pattern(place_builder, subpattern, cx)); + MatchPairTree::for_pattern(place_builder, subpattern, cx, &mut subpairs); TestCase::Irrefutable { ascription, binding: None } } @@ -156,13 +158,13 @@ impl<'tcx> MatchPairTree<'tcx> { if let Some(subpattern) = subpattern.as_ref() { // this is the `x @ P` case; have to keep matching against `P` now - subpairs.push(MatchPairTree::for_pattern(place_builder, subpattern, cx)); + MatchPairTree::for_pattern(place_builder, subpattern, cx, &mut subpairs); } TestCase::Irrefutable { ascription: None, binding } } PatKind::ExpandedConstant { subpattern: ref pattern, def_id: _, is_inline: false } => { - subpairs.push(MatchPairTree::for_pattern(place_builder, pattern, cx)); + MatchPairTree::for_pattern(place_builder, pattern, cx, &mut subpairs); default_irrefutable() } PatKind::ExpandedConstant { subpattern: ref pattern, def_id, is_inline: true } => { @@ -189,7 +191,7 @@ impl<'tcx> MatchPairTree<'tcx> { super::Ascription { annotation, source, variance: ty::Contravariant } }); - subpairs.push(MatchPairTree::for_pattern(place_builder, pattern, cx)); + MatchPairTree::for_pattern(place_builder, pattern, cx, &mut subpairs); TestCase::Irrefutable { ascription, binding: None } } @@ -235,7 +237,7 @@ impl<'tcx> MatchPairTree<'tcx> { } PatKind::Deref { ref subpattern } => { - subpairs.push(MatchPairTree::for_pattern(place_builder.deref(), subpattern, cx)); + MatchPairTree::for_pattern(place_builder.deref(), subpattern, cx, &mut subpairs); default_irrefutable() } @@ -246,23 +248,24 @@ impl<'tcx> MatchPairTree<'tcx> { Ty::new_ref(cx.tcx, cx.tcx.lifetimes.re_erased, subpattern.ty, mutability), pattern.span, ); - subpairs.push(MatchPairTree::for_pattern( + MatchPairTree::for_pattern( PlaceBuilder::from(temp).deref(), subpattern, cx, - )); + &mut subpairs, + ); TestCase::Deref { temp, mutability } } PatKind::Never => TestCase::Never, }; - MatchPairTree { + match_pairs.push(MatchPairTree { place, test_case, subpairs, pattern_ty: pattern.ty, pattern_span: pattern.span, - } + }); } } diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index d05d5b151ff48..e41001193d54c 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -1009,7 +1009,8 @@ impl<'tcx> FlatPat<'tcx> { /// for the given pattern. fn new(place: PlaceBuilder<'tcx>, pattern: &Pat<'tcx>, cx: &mut Builder<'_, 'tcx>) -> Self { // First, recursively build a tree of match pairs for the given pattern. - let mut match_pairs = vec![MatchPairTree::for_pattern(place, pattern, cx)]; + let mut match_pairs = vec![]; + MatchPairTree::for_pattern(place, pattern, cx, &mut match_pairs); let mut extra_data = PatternExtraData { span: pattern.span, bindings: Vec::new(), From ef442738386e8daf318f177bd6a551529e99b0da Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 5 Mar 2025 23:25:59 +1100 Subject: [PATCH 3/6] Populate pattern bindings/ascriptions while building `MatchPairTree` --- .../src/builder/matches/match_pair.rs | 130 +++++++++++++----- .../src/builder/matches/mod.rs | 8 +- .../src/builder/matches/simplify.rs | 8 +- 3 files changed, 100 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs index 7252dadc3202a..89d8189cee03a 100644 --- a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs +++ b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs @@ -6,7 +6,7 @@ use rustc_middle::ty::{self, Ty, TypeVisitableExt}; use crate::builder::Builder; use crate::builder::expr::as_place::{PlaceBase, PlaceBuilder}; -use crate::builder::matches::{FlatPat, MatchPairTree, TestCase}; +use crate::builder::matches::{FlatPat, MatchPairTree, PatternExtraData, TestCase}; impl<'a, 'tcx> Builder<'a, 'tcx> { /// Builds and pushes [`MatchPairTree`] subtrees, one for each pattern in @@ -17,12 +17,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn field_match_pairs( &mut self, match_pairs: &mut Vec>, + extra_data: &mut PatternExtraData<'tcx>, place: PlaceBuilder<'tcx>, subpatterns: &[FieldPat<'tcx>], ) { for fieldpat in subpatterns { let place = place.clone_project(PlaceElem::Field(fieldpat.field, fieldpat.pattern.ty)); - MatchPairTree::for_pattern(place, &fieldpat.pattern, self, match_pairs); + MatchPairTree::for_pattern(place, &fieldpat.pattern, self, match_pairs, extra_data); } } @@ -33,6 +34,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn prefix_slice_suffix( &mut self, match_pairs: &mut Vec>, + extra_data: &mut PatternExtraData<'tcx>, place: &PlaceBuilder<'tcx>, prefix: &[Pat<'tcx>], opt_slice: &Option>>, @@ -57,7 +59,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let elem = ProjectionElem::ConstantIndex { offset: idx as u64, min_length, from_end: false }; let place = place.clone_project(elem); - MatchPairTree::for_pattern(place, subpattern, self, match_pairs); + MatchPairTree::for_pattern(place, subpattern, self, match_pairs, extra_data) } if let Some(subslice_pat) = opt_slice { @@ -67,7 +69,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { to: if exact_size { min_length - suffix_len } else { suffix_len }, from_end: !exact_size, }); - MatchPairTree::for_pattern(subslice, subslice_pat, self, match_pairs); + MatchPairTree::for_pattern(subslice, subslice_pat, self, match_pairs, extra_data); } for (idx, subpattern) in suffix.iter().rev().enumerate() { @@ -78,7 +80,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { from_end: !exact_size, }; let place = place.clone_project(elem); - MatchPairTree::for_pattern(place, subpattern, self, match_pairs); + MatchPairTree::for_pattern(place, subpattern, self, match_pairs, extra_data) } } } @@ -86,11 +88,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { impl<'tcx> MatchPairTree<'tcx> { /// Recursively builds a match pair tree for the given pattern and its /// subpatterns. - pub(in crate::builder) fn for_pattern( + pub(super) fn for_pattern( mut place_builder: PlaceBuilder<'tcx>, pattern: &Pat<'tcx>, cx: &mut Builder<'_, 'tcx>, match_pairs: &mut Vec, // Newly-created nodes are added to this vector + extra_data: &mut PatternExtraData<'tcx>, // Bindings/ascriptions are added here ) { // Force the place type to the pattern's type. // FIXME(oli-obk): can we use this to simplify slice/array pattern hacks? @@ -113,7 +116,7 @@ impl<'tcx> MatchPairTree<'tcx> { } let place = place_builder.try_to_place(cx); - let default_irrefutable = || TestCase::Irrefutable { binding: None, ascription: None }; + let default_irrefutable = || TestCase::Irrefutable {}; let mut subpairs = Vec::new(); let test_case = match pattern.kind { PatKind::Wild | PatKind::Error(_) => default_irrefutable(), @@ -137,39 +140,77 @@ impl<'tcx> MatchPairTree<'tcx> { ref subpattern, .. } => { + MatchPairTree::for_pattern( + place_builder, + subpattern, + cx, + &mut subpairs, + extra_data, + ); + // Apply the type ascription to the value at `match_pair.place` - let ascription = place.map(|source| super::Ascription { - annotation: annotation.clone(), - source, - variance, - }); - - MatchPairTree::for_pattern(place_builder, subpattern, cx, &mut subpairs); - TestCase::Irrefutable { ascription, binding: None } + if let Some(source) = place { + let annotation = annotation.clone(); + extra_data.ascriptions.push(super::Ascription { source, annotation, variance }); + } + + default_irrefutable() } PatKind::Binding { mode, var, ref subpattern, .. } => { - let binding = place.map(|source| super::Binding { - span: pattern.span, - source, - var_id: var, - binding_mode: mode, - }); + // In order to please the borrow checker, when lowering a pattern + // like `x @ subpat` we must establish any bindings in `subpat` + // before establishing the binding for `x`. + // + // For example (from #69971): + // + // ```ignore (illustrative) + // struct NonCopyStruct { + // copy_field: u32, + // } + // + // fn foo1(x: NonCopyStruct) { + // let y @ NonCopyStruct { copy_field: z } = x; + // // the above should turn into + // let z = x.copy_field; + // let y = x; + // } + // ``` + // First, recurse into the subpattern, if any. if let Some(subpattern) = subpattern.as_ref() { // this is the `x @ P` case; have to keep matching against `P` now - MatchPairTree::for_pattern(place_builder, subpattern, cx, &mut subpairs); + MatchPairTree::for_pattern( + place_builder, + subpattern, + cx, + &mut subpairs, + extra_data, + ); + } + + // Then push this binding, after any bindings in the subpattern. + if let Some(source) = place { + extra_data.bindings.push(super::Binding { + span: pattern.span, + source, + var_id: var, + binding_mode: mode, + }); } - TestCase::Irrefutable { ascription: None, binding } + + default_irrefutable() } PatKind::ExpandedConstant { subpattern: ref pattern, def_id: _, is_inline: false } => { - MatchPairTree::for_pattern(place_builder, pattern, cx, &mut subpairs); + MatchPairTree::for_pattern(place_builder, pattern, cx, &mut subpairs, extra_data); default_irrefutable() } PatKind::ExpandedConstant { subpattern: ref pattern, def_id, is_inline: true } => { + MatchPairTree::for_pattern(place_builder, pattern, cx, &mut subpairs, extra_data); + // Apply a type ascription for the inline constant to the value at `match_pair.place` - let ascription = place.map(|source| { + if let Some(source) = place { let span = pattern.span; let parent_id = cx.tcx.typeck_root_def_id(cx.def_id.to_def_id()); let args = ty::InlineConstArgs::new( @@ -188,19 +229,33 @@ impl<'tcx> MatchPairTree<'tcx> { span, user_ty: Box::new(user_ty), }; - super::Ascription { annotation, source, variance: ty::Contravariant } - }); + let variance = ty::Contravariant; + extra_data.ascriptions.push(super::Ascription { annotation, source, variance }); + } - MatchPairTree::for_pattern(place_builder, pattern, cx, &mut subpairs); - TestCase::Irrefutable { ascription, binding: None } + default_irrefutable() } PatKind::Array { ref prefix, ref slice, ref suffix } => { - cx.prefix_slice_suffix(&mut subpairs, &place_builder, prefix, slice, suffix); + cx.prefix_slice_suffix( + &mut subpairs, + extra_data, + &place_builder, + prefix, + slice, + suffix, + ); default_irrefutable() } PatKind::Slice { ref prefix, ref slice, ref suffix } => { - cx.prefix_slice_suffix(&mut subpairs, &place_builder, prefix, slice, suffix); + cx.prefix_slice_suffix( + &mut subpairs, + extra_data, + &place_builder, + prefix, + slice, + suffix, + ); if prefix.is_empty() && slice.is_some() && suffix.is_empty() { default_irrefutable() @@ -214,7 +269,7 @@ impl<'tcx> MatchPairTree<'tcx> { PatKind::Variant { adt_def, variant_index, args, ref subpatterns } => { let downcast_place = place_builder.downcast(adt_def, variant_index); // `(x as Variant)` - cx.field_match_pairs(&mut subpairs, downcast_place, subpatterns); + cx.field_match_pairs(&mut subpairs, extra_data, downcast_place, subpatterns); let irrefutable = adt_def.variants().iter_enumerated().all(|(i, v)| { i == variant_index @@ -232,12 +287,18 @@ impl<'tcx> MatchPairTree<'tcx> { } PatKind::Leaf { ref subpatterns } => { - cx.field_match_pairs(&mut subpairs, place_builder, subpatterns); + cx.field_match_pairs(&mut subpairs, extra_data, place_builder, subpatterns); default_irrefutable() } PatKind::Deref { ref subpattern } => { - MatchPairTree::for_pattern(place_builder.deref(), subpattern, cx, &mut subpairs); + MatchPairTree::for_pattern( + place_builder.deref(), + subpattern, + cx, + &mut subpairs, + extra_data, + ); default_irrefutable() } @@ -253,6 +314,7 @@ impl<'tcx> MatchPairTree<'tcx> { subpattern, cx, &mut subpairs, + extra_data, ); TestCase::Deref { temp, mutability } } diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index e41001193d54c..8c8f4c3c92c0f 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -1008,17 +1008,15 @@ impl<'tcx> FlatPat<'tcx> { /// Creates a `FlatPat` containing a simplified [`MatchPairTree`] list/forest /// for the given pattern. fn new(place: PlaceBuilder<'tcx>, pattern: &Pat<'tcx>, cx: &mut Builder<'_, 'tcx>) -> Self { - // First, recursively build a tree of match pairs for the given pattern. + // Recursively build a tree of match pairs for the given pattern. let mut match_pairs = vec![]; - MatchPairTree::for_pattern(place, pattern, cx, &mut match_pairs); let mut extra_data = PatternExtraData { span: pattern.span, bindings: Vec::new(), ascriptions: Vec::new(), is_never: pattern.is_never_pattern(), }; - // Recursively remove irrefutable match pairs, while recording their - // bindings/ascriptions, and sort or-patterns after other match pairs. + MatchPairTree::for_pattern(place, pattern, cx, &mut match_pairs, &mut extra_data); cx.simplify_match_pairs(&mut match_pairs, &mut extra_data); Self { match_pairs, extra_data } @@ -1238,7 +1236,7 @@ struct Ascription<'tcx> { /// - See [`Builder::expand_and_match_or_candidates`]. #[derive(Debug, Clone)] enum TestCase<'tcx> { - Irrefutable { binding: Option>, ascription: Option> }, + Irrefutable {}, Variant { adt_def: ty::AdtDef<'tcx>, variant_index: VariantIdx }, Constant { value: mir::Const<'tcx> }, Range(Arc>), diff --git a/compiler/rustc_mir_build/src/builder/matches/simplify.rs b/compiler/rustc_mir_build/src/builder/matches/simplify.rs index 15c860151dc9e..f4bcf62a4b1f7 100644 --- a/compiler/rustc_mir_build/src/builder/matches/simplify.rs +++ b/compiler/rustc_mir_build/src/builder/matches/simplify.rs @@ -47,13 +47,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // match lowering forces us to lower bindings inside or-patterns last. for mut match_pair in mem::take(match_pairs) { self.simplify_match_pairs(&mut match_pair.subpairs, extra_data); - if let TestCase::Irrefutable { binding, ascription } = match_pair.test_case { - if let Some(binding) = binding { - extra_data.bindings.push(binding); - } - if let Some(ascription) = ascription { - extra_data.ascriptions.push(ascription); - } + if let TestCase::Irrefutable {} = match_pair.test_case { // Simplifiable pattern; we replace it with its already simplified subpairs. match_pairs.append(&mut match_pair.subpairs); } else { From 854feae88773da49f2b1a900f46668938cf52b85 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 5 Mar 2025 23:25:59 +1100 Subject: [PATCH 4/6] Remove `TestCase::Irrefutable` --- .../src/builder/matches/match_pair.rs | 64 ++++++++++--------- .../src/builder/matches/mod.rs | 17 +---- .../src/builder/matches/simplify.rs | 9 +-- .../src/builder/matches/test.rs | 6 -- 4 files changed, 38 insertions(+), 58 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs index 89d8189cee03a..55487f48509ff 100644 --- a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs +++ b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs @@ -116,24 +116,23 @@ impl<'tcx> MatchPairTree<'tcx> { } let place = place_builder.try_to_place(cx); - let default_irrefutable = || TestCase::Irrefutable {}; let mut subpairs = Vec::new(); let test_case = match pattern.kind { - PatKind::Wild | PatKind::Error(_) => default_irrefutable(), + PatKind::Wild | PatKind::Error(_) => None, - PatKind::Or { ref pats } => TestCase::Or { + PatKind::Or { ref pats } => Some(TestCase::Or { pats: pats.iter().map(|pat| FlatPat::new(place_builder.clone(), pat, cx)).collect(), - }, + }), PatKind::Range(ref range) => { if range.is_full_range(cx.tcx) == Some(true) { - default_irrefutable() + None } else { - TestCase::Range(Arc::clone(range)) + Some(TestCase::Range(Arc::clone(range))) } } - PatKind::Constant { value } => TestCase::Constant { value }, + PatKind::Constant { value } => Some(TestCase::Constant { value }), PatKind::AscribeUserType { ascription: Ascription { ref annotation, variance }, @@ -154,7 +153,7 @@ impl<'tcx> MatchPairTree<'tcx> { extra_data.ascriptions.push(super::Ascription { source, annotation, variance }); } - default_irrefutable() + None } PatKind::Binding { mode, var, ref subpattern, .. } => { @@ -199,12 +198,12 @@ impl<'tcx> MatchPairTree<'tcx> { }); } - default_irrefutable() + None } PatKind::ExpandedConstant { subpattern: ref pattern, def_id: _, is_inline: false } => { MatchPairTree::for_pattern(place_builder, pattern, cx, &mut subpairs, extra_data); - default_irrefutable() + None } PatKind::ExpandedConstant { subpattern: ref pattern, def_id, is_inline: true } => { MatchPairTree::for_pattern(place_builder, pattern, cx, &mut subpairs, extra_data); @@ -233,7 +232,7 @@ impl<'tcx> MatchPairTree<'tcx> { extra_data.ascriptions.push(super::Ascription { annotation, source, variance }); } - default_irrefutable() + None } PatKind::Array { ref prefix, ref slice, ref suffix } => { @@ -245,7 +244,7 @@ impl<'tcx> MatchPairTree<'tcx> { slice, suffix, ); - default_irrefutable() + None } PatKind::Slice { ref prefix, ref slice, ref suffix } => { cx.prefix_slice_suffix( @@ -258,12 +257,12 @@ impl<'tcx> MatchPairTree<'tcx> { ); if prefix.is_empty() && slice.is_some() && suffix.is_empty() { - default_irrefutable() + None } else { - TestCase::Slice { + Some(TestCase::Slice { len: prefix.len() + suffix.len(), variable_length: slice.is_some(), - } + }) } } @@ -279,16 +278,12 @@ impl<'tcx> MatchPairTree<'tcx> { .apply_ignore_module(cx.tcx, cx.infcx.typing_env(cx.param_env)) }) && (adt_def.did().is_local() || !adt_def.is_variant_list_non_exhaustive()); - if irrefutable { - default_irrefutable() - } else { - TestCase::Variant { adt_def, variant_index } - } + if irrefutable { None } else { Some(TestCase::Variant { adt_def, variant_index }) } } PatKind::Leaf { ref subpatterns } => { cx.field_match_pairs(&mut subpairs, extra_data, place_builder, subpatterns); - default_irrefutable() + None } PatKind::Deref { ref subpattern } => { @@ -299,7 +294,7 @@ impl<'tcx> MatchPairTree<'tcx> { &mut subpairs, extra_data, ); - default_irrefutable() + None } PatKind::DerefPattern { ref subpattern, mutability } => { @@ -316,18 +311,25 @@ impl<'tcx> MatchPairTree<'tcx> { &mut subpairs, extra_data, ); - TestCase::Deref { temp, mutability } + Some(TestCase::Deref { temp, mutability }) } - PatKind::Never => TestCase::Never, + PatKind::Never => Some(TestCase::Never), }; - match_pairs.push(MatchPairTree { - place, - test_case, - subpairs, - pattern_ty: pattern.ty, - pattern_span: pattern.span, - }); + if let Some(test_case) = test_case { + // This pattern is refutable, so push a new match-pair node. + match_pairs.push(MatchPairTree { + place, + test_case, + subpairs, + pattern_ty: pattern.ty, + pattern_span: pattern.span, + }) + } else { + // This pattern is irrefutable, so it doesn't need its own match-pair node. + // Just push its refutable subpatterns instead, if any. + match_pairs.extend(subpairs); + } } } diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index 8c8f4c3c92c0f..bb93482c0b6ce 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -1054,7 +1054,6 @@ struct Candidate<'tcx> { /// (see [`Builder::test_remaining_match_pairs_after_or`]). /// /// Invariants: - /// - All [`TestCase::Irrefutable`] patterns have been removed by simplification. /// - All or-patterns ([`TestCase::Or`]) have been sorted to the end. match_pairs: Vec>, @@ -1226,17 +1225,11 @@ struct Ascription<'tcx> { /// - [`Builder::pick_test_for_match_pair`] (to choose a test) /// - [`Builder::sort_candidate`] (to see how the test interacts with a match pair) /// -/// Two variants are unlike the others and deserve special mention: -/// -/// - [`Self::Irrefutable`] is only used temporarily when building a [`MatchPairTree`]. -/// They are then flattened away by [`Builder::simplify_match_pairs`], with any -/// bindings/ascriptions incorporated into the enclosing [`FlatPat`]. -/// - [`Self::Or`] are not tested directly like the other variants. Instead they -/// participate in or-pattern expansion, where they are transformed into subcandidates. -/// - See [`Builder::expand_and_match_or_candidates`]. +/// Note that or-patterns are not tested directly like the other variants. +/// Instead they participate in or-pattern expansion, where they are transformed into +/// subcandidates. See [`Builder::expand_and_match_or_candidates`]. #[derive(Debug, Clone)] enum TestCase<'tcx> { - Irrefutable {}, Variant { adt_def: ty::AdtDef<'tcx>, variant_index: VariantIdx }, Constant { value: mir::Const<'tcx> }, Range(Arc>), @@ -1269,10 +1262,6 @@ pub(crate) struct MatchPairTree<'tcx> { place: Option>, /// ... must pass this test... - /// - /// --- - /// Invariant: after creation and simplification in [`FlatPat::new`], - /// this must not be [`TestCase::Irrefutable`]. test_case: TestCase<'tcx>, /// ... and these subpairs must match. diff --git a/compiler/rustc_mir_build/src/builder/matches/simplify.rs b/compiler/rustc_mir_build/src/builder/matches/simplify.rs index f4bcf62a4b1f7..5965b35b70c0f 100644 --- a/compiler/rustc_mir_build/src/builder/matches/simplify.rs +++ b/compiler/rustc_mir_build/src/builder/matches/simplify.rs @@ -47,13 +47,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // match lowering forces us to lower bindings inside or-patterns last. for mut match_pair in mem::take(match_pairs) { self.simplify_match_pairs(&mut match_pair.subpairs, extra_data); - if let TestCase::Irrefutable {} = match_pair.test_case { - // Simplifiable pattern; we replace it with its already simplified subpairs. - match_pairs.append(&mut match_pair.subpairs); - } else { - // Unsimplifiable pattern; we keep it. - match_pairs.push(match_pair); - } + // Unsimplifiable pattern; we keep it. + match_pairs.push(match_pair); } // Move or-patterns to the end, because they can result in us diff --git a/compiler/rustc_mir_build/src/builder/matches/test.rs b/compiler/rustc_mir_build/src/builder/matches/test.rs index a7a028bff97f7..fe5ec32422cf3 100644 --- a/compiler/rustc_mir_build/src/builder/matches/test.rs +++ b/compiler/rustc_mir_build/src/builder/matches/test.rs @@ -55,12 +55,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Or-patterns are not tested directly; instead they are expanded into subcandidates, // which are then distinguished by testing whatever non-or patterns they contain. TestCase::Or { .. } => bug!("or-patterns should have already been handled"), - - TestCase::Irrefutable { .. } => span_bug!( - match_pair.pattern_span, - "simplifiable pattern found: {:?}", - match_pair.pattern_span - ), }; Test { span: match_pair.pattern_span, kind } From e05df1cb5d13200314e95dde3e0f497d6c5a38aa Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 5 Mar 2025 23:26:00 +1100 Subject: [PATCH 5/6] Remove the separate simplify step for match-pair trees What remained of this simplification process has been integrated into construction of the match-pair trees. --- .../src/builder/matches/mod.rs | 23 +++---- .../src/builder/matches/simplify.rs | 60 ------------------- .../src/builder/matches/test.rs | 2 +- 3 files changed, 14 insertions(+), 71 deletions(-) delete mode 100644 compiler/rustc_mir_build/src/builder/matches/simplify.rs diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index bb93482c0b6ce..5d97bf13f8692 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -26,7 +26,6 @@ use crate::builder::{ // helper functions, broken out by category: mod match_pair; -mod simplify; mod test; mod util; @@ -987,18 +986,16 @@ impl<'tcx> PatternExtraData<'tcx> { } /// A pattern in a form suitable for lowering the match tree, with all irrefutable -/// patterns simplified away, and or-patterns sorted to the end. +/// patterns simplified away. /// -/// 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. +/// Here, "flat" indicates that irrefutable nodes in the pattern tree have been +/// recursively replaced with their refutable subpatterns. They are not +/// necessarily flat in an absolute sense. /// /// Will typically be incorporated into a [`Candidate`]. #[derive(Debug, Clone)] struct FlatPat<'tcx> { /// To match the pattern, all of these must be satisfied... - // Invariant: all the match pairs are recursively simplified. - // Invariant: or-patterns must be sorted to the end. match_pairs: Vec>, extra_data: PatternExtraData<'tcx>, @@ -1017,7 +1014,6 @@ impl<'tcx> FlatPat<'tcx> { is_never: pattern.is_never_pattern(), }; MatchPairTree::for_pattern(place, pattern, cx, &mut match_pairs, &mut extra_data); - cx.simplify_match_pairs(&mut match_pairs, &mut extra_data); Self { match_pairs, extra_data } } @@ -1124,7 +1120,7 @@ impl<'tcx> Candidate<'tcx> { /// Incorporates an already-simplified [`FlatPat`] into a new candidate. fn from_flat_pat(flat_pat: FlatPat<'tcx>, has_guard: bool) -> Self { - Candidate { + let mut this = Candidate { match_pairs: flat_pat.match_pairs, extra_data: flat_pat.extra_data, has_guard, @@ -1133,7 +1129,14 @@ impl<'tcx> Candidate<'tcx> { otherwise_block: None, pre_binding_block: None, false_edge_start_block: None, - } + }; + this.sort_match_pairs(); + this + } + + /// Restores the invariant that or-patterns must be sorted to the end. + fn sort_match_pairs(&mut self) { + self.match_pairs.sort_by_key(|pair| matches!(pair.test_case, TestCase::Or { .. })); } /// Returns whether the first match pair of this candidate is an or-pattern. diff --git a/compiler/rustc_mir_build/src/builder/matches/simplify.rs b/compiler/rustc_mir_build/src/builder/matches/simplify.rs deleted file mode 100644 index 5965b35b70c0f..0000000000000 --- a/compiler/rustc_mir_build/src/builder/matches/simplify.rs +++ /dev/null @@ -1,60 +0,0 @@ -//! Simplifying Candidates -//! -//! *Simplifying* a match pair `place @ pattern` means breaking it down -//! into bindings or other, simpler match pairs. For example: -//! -//! - `place @ (P1, P2)` can be simplified to `[place.0 @ P1, place.1 @ P2]` -//! - `place @ x` can be simplified to `[]` by binding `x` to `place` -//! -//! The `simplify_match_pairs` routine just repeatedly applies these -//! sort of simplifications until there is nothing left to -//! simplify. Match pairs cannot be simplified if they require some -//! sort of test: for example, testing which variant an enum is, or -//! testing a value against a constant. - -use std::mem; - -use tracing::{debug, instrument}; - -use crate::builder::Builder; -use crate::builder::matches::{MatchPairTree, PatternExtraData, TestCase}; - -impl<'a, 'tcx> Builder<'a, 'tcx> { - /// Simplify a list of match pairs so they all require a test. Stores relevant bindings and - /// ascriptions in `extra_data`. - #[instrument(skip(self), level = "debug")] - pub(super) fn simplify_match_pairs( - &mut self, - match_pairs: &mut Vec>, - extra_data: &mut PatternExtraData<'tcx>, - ) { - // In order to please the borrow checker, in a pattern like `x @ pat` we must lower the - // bindings in `pat` before `x`. E.g. (#69971): - // - // struct NonCopyStruct { - // copy_field: u32, - // } - // - // fn foo1(x: NonCopyStruct) { - // let y @ NonCopyStruct { copy_field: z } = x; - // // the above should turn into - // let z = x.copy_field; - // let y = x; - // } - // - // We therefore lower bindings from left-to-right, except we lower the `x` in `x @ pat` - // after any bindings in `pat`. This doesn't work for or-patterns: the current structure of - // match lowering forces us to lower bindings inside or-patterns last. - for mut match_pair in mem::take(match_pairs) { - self.simplify_match_pairs(&mut match_pair.subpairs, extra_data); - // Unsimplifiable pattern; we keep it. - match_pairs.push(match_pair); - } - - // Move or-patterns to the end, because they can result in us - // creating additional candidates, so we want to test them as - // late as possible. - match_pairs.sort_by_key(|pair| matches!(pair.test_case, TestCase::Or { .. })); - debug!(simplified = ?match_pairs, "simplify_match_pairs"); - } -} diff --git a/compiler/rustc_mir_build/src/builder/matches/test.rs b/compiler/rustc_mir_build/src/builder/matches/test.rs index fe5ec32422cf3..f92036a83e1ef 100644 --- a/compiler/rustc_mir_build/src/builder/matches/test.rs +++ b/compiler/rustc_mir_build/src/builder/matches/test.rs @@ -763,7 +763,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let match_pair = candidate.match_pairs.remove(match_pair_index); candidate.match_pairs.extend(match_pair.subpairs); // Move or-patterns to the end. - candidate.match_pairs.sort_by_key(|pair| matches!(pair.test_case, TestCase::Or { .. })); + candidate.sort_match_pairs(); } ret From e3e74bc89a958e36c658fa809d98b4dd66d53cf8 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 5 Mar 2025 23:26:00 +1100 Subject: [PATCH 6/6] Make `MatchPairTree::place` non-optional As the invariant indicated, this place could only be none for `TestCase::Irrefutable` nodes, which no longer exist. --- .../src/builder/matches/match_pair.rs | 3 ++- compiler/rustc_mir_build/src/builder/matches/mod.rs | 12 ++---------- compiler/rustc_mir_build/src/builder/matches/test.rs | 9 +++------ compiler/rustc_mir_build/src/builder/matches/util.rs | 8 ++------ 4 files changed, 9 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs index 55487f48509ff..c6f3e22e95f6f 100644 --- a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs +++ b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs @@ -115,6 +115,7 @@ impl<'tcx> MatchPairTree<'tcx> { place_builder = place_builder.project(ProjectionElem::OpaqueCast(pattern.ty)); } + // Place can be none if the pattern refers to a non-captured place in a closure. let place = place_builder.try_to_place(cx); let mut subpairs = Vec::new(); let test_case = match pattern.kind { @@ -320,7 +321,7 @@ impl<'tcx> MatchPairTree<'tcx> { if let Some(test_case) = test_case { // This pattern is refutable, so push a new match-pair node. match_pairs.push(MatchPairTree { - place, + place: place.expect("refutable patterns should always have a place to inspect"), test_case, subpairs, pattern_ty: pattern.ty, diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index 5d97bf13f8692..b05052a3455ea 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -1256,13 +1256,7 @@ impl<'tcx> TestCase<'tcx> { #[derive(Debug, Clone)] pub(crate) struct MatchPairTree<'tcx> { /// This place... - /// - /// --- - /// This can be `None` if it referred to a non-captured place in a closure. - /// - /// Invariant: Can only be `None` when `test_case` is `Irrefutable`. - /// Therefore this must be `Some(_)` after simplification. - place: Option>, + place: Place<'tcx>, /// ... must pass this test... test_case: TestCase<'tcx>, @@ -2082,11 +2076,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Extract the match-pair from the highest priority candidate let match_pair = &candidates[0].match_pairs[0]; let test = self.pick_test_for_match_pair(match_pair); - // Unwrap is ok after simplification. - let match_place = match_pair.place.unwrap(); debug!(?test, ?match_pair); - (match_place, test) + (match_pair.place, test) } /// Given a test, we partition the input candidates into several buckets. diff --git a/compiler/rustc_mir_build/src/builder/matches/test.rs b/compiler/rustc_mir_build/src/builder/matches/test.rs index f92036a83e1ef..e5d61bc9e556a 100644 --- a/compiler/rustc_mir_build/src/builder/matches/test.rs +++ b/compiler/rustc_mir_build/src/builder/matches/test.rs @@ -540,11 +540,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // than one, but it'd be very unusual to have two sides that // both require tests; you'd expect one side to be simplified // away.) - let (match_pair_index, match_pair) = candidate - .match_pairs - .iter() - .enumerate() - .find(|&(_, mp)| mp.place == Some(test_place))?; + let (match_pair_index, match_pair) = + candidate.match_pairs.iter().enumerate().find(|&(_, mp)| mp.place == test_place)?; // If true, the match pair is completely entailed by its corresponding test // branch, so it can be removed. If false, the match pair is _compatible_ @@ -587,7 +584,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate .match_pairs .iter() - .any(|mp| mp.place == Some(test_place) && is_covering_range(&mp.test_case)) + .any(|mp| mp.place == test_place && is_covering_range(&mp.test_case)) }; if sorted_candidates .get(&TestBranch::Failure) diff --git a/compiler/rustc_mir_build/src/builder/matches/util.rs b/compiler/rustc_mir_build/src/builder/matches/util.rs index 589e350a03fc3..ed3b652e4ef5d 100644 --- a/compiler/rustc_mir_build/src/builder/matches/util.rs +++ b/compiler/rustc_mir_build/src/builder/matches/util.rs @@ -173,14 +173,10 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { // } // ``` // Hence we fake borrow using a deep borrow. - if let Some(place) = match_pair.place { - self.fake_borrow(place, FakeBorrowKind::Deep); - } + self.fake_borrow(match_pair.place, FakeBorrowKind::Deep); } else { // Insert a Shallow borrow of any place that is switched on. - if let Some(place) = match_pair.place { - self.fake_borrow(place, FakeBorrowKind::Shallow); - } + self.fake_borrow(match_pair.place, FakeBorrowKind::Shallow); for subpair in &match_pair.subpairs { self.visit_match_pair(subpair);