From ac609f98e80b25b27f779095e24a0ab726ec0361 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 2 Aug 2026 21:30:02 +1000 Subject: [PATCH 1/2] Distinguish or/refutable/irrefutable patterns in `InterPat` --- .../src/builder/matches/match_pair.rs | 322 +++++++++--------- 1 file changed, 165 insertions(+), 157 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 b4ce8149f5e4d..2dcfbf3ca3098 100644 --- a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs +++ b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs @@ -107,113 +107,81 @@ fn squash_inter_pat<'tcx>( extra_data: &mut PatternExtraData<'tcx>, // Bindings/ascriptions are added here ) { // Destructure exhaustively to make sure we don't miss any fields. - let InterPat { - place, - testable_case, - subpats, - or_subpats, - ascriptions, - binding, - pattern_span, - is_never: _, // Not needed by `MatchPairTree` forests. - } = inter_pat; + // The `is_never` field is not needed by `MatchPairTree` forests. + let InterPat { kind, ascriptions, pattern_span, is_never: _ } = inter_pat; // Type ascriptions can appear regardless of whether the node is an or-pattern. extra_data.ascriptions.extend(ascriptions); - // Or and non-or patterns have very different handling. - if let Some(or_subpats) = or_subpats { - // We're dealing with an or-pattern node. - assert!(testable_case.is_none()); - assert!(subpats.is_empty()); - assert!(binding.is_none()); - - let or_subpats = or_subpats - .into_iter() - .map(|subpat| FlatPat::from_inter_pat(subpat)) - .collect::>(); - - if !or_subpats[0].extra_data.bindings.is_empty() { - // Hold a place for any bindings established in (possibly-nested) or-patterns. - // By only holding a place when bindings are present, we skip over any - // or-patterns that will be simplified by `merge_trivial_subcandidates`. In - // other words, we can assume this expands into subcandidates. - // FIXME(@dianne): this needs updating/removing if we always merge or-patterns - extra_data.bindings.push(super::SubpatternBindings::FromOrPattern); - } + // Or patterns, refutable patterns, and irrefutable patterns all have different handling. + match kind { + InterPatKind::Or { or_subpats } => { + let or_subpats = or_subpats + .into_iter() + .map(|subpat| FlatPat::from_inter_pat(subpat)) + .collect::>(); + + if !or_subpats[0].extra_data.bindings.is_empty() { + // Hold a place for any bindings established in (possibly-nested) or-patterns. + // By only holding a place when bindings are present, we skip over any + // or-patterns that will be simplified by `merge_trivial_subcandidates`. In + // other words, we can assume this expands into subcandidates. + // FIXME(@dianne): this needs updating/removing if we always merge or-patterns + extra_data.bindings.push(super::SubpatternBindings::FromOrPattern); + } - match_pairs.push(MatchPairTree { - // Or-patterns never need a place during MIR building. - place: None, - testable_case: TestableCase::Or { pats: or_subpats }, - subpairs: vec![], - pattern_span, - }); - } else { - // We're dealing with a node that isn't an or-pattern. - - // Recursively squash any subpatterns into refutable `MatchPairTree` forests. - // This must happen _before_ pushing the binding, as described by the binding step. - let mut subpairs = vec![]; - for subpat in subpats { - squash_inter_pat(subpat, &mut subpairs, extra_data); + match_pairs.push(MatchPairTree { + // Or-patterns never need a place during MIR building. + place: None, + testable_case: TestableCase::Or { pats: or_subpats }, + subpairs: vec![], + pattern_span, + }); } - if let Some(testable_case) = testable_case { + InterPatKind::Refutable { place, testable_case, subpats } => { + // Recursively squash any subpatterns into refutable `MatchPairTree` forests, + // which will become the children of a new node. + let mut subpairs = vec![]; + for subpat in subpats { + squash_inter_pat(subpat, &mut subpairs, extra_data); + } + // This pattern is refutable, so push a new match-pair node. - // - // If this match is inside a closure, it's essential that the place - // we're testing was actually captured! Be sure to keep `ExprUseVisitor` - // in sync with the refutability checks in this module. - assert!(place.is_some()); assert!(!matches!(testable_case, TestableCase::Or { .. })); - match_pairs.push(MatchPairTree { place, testable_case, subpairs, 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); + match_pairs.push(MatchPairTree { + place: Some(place), + testable_case, + subpairs, + pattern_span, + }); } - // If present, the binding must be pushed _after_ traversing subpatterns. - // This is so that when lowering something like `x @ NonCopy { copy_field }`, - // the binding to `copy_field` will occur before the binding for `x`. - // See for more background. - if let Some(binding) = binding { - extra_data.bindings.push(super::SubpatternBindings::One(binding)); + InterPatKind::Irrefutable { subpats, binding } => { + // Recursively squash any subpatterns into refutable `MatchPairTree` forests. + // This must happen _before_ pushing the binding, as described by the binding step. + for subpat in subpats { + // For irrefutable nodes, squash directly into the caller's match pairs. + squash_inter_pat(subpat, match_pairs, extra_data); + } + + // If present, the binding must be pushed _after_ traversing subpatterns. + // This is so that when lowering something like `x @ NonCopy { copy_field }`, + // the binding to `copy_field` will occur before the binding for `x`. + // See for more background. + if let Some(binding) = binding { + extra_data.bindings.push(super::SubpatternBindings::One(binding)); + } } } } /// "Intermediate pattern", a partly-lowered THIR [`Pat`] that has not yet been /// squashed into a forest of refutable [`MatchPairTree`] nodes. -/// -/// FIXME(Zalathar): This could potentially be split into different enum variants -/// for or-patterns and non-or patterns, but for now the flat structure makes -/// construction a bit easier, at the cost of more complicated invariants. struct InterPat<'tcx> { - /// Place that this pattern node will test. - /// - /// If `None`, we're in a closure that didn't capture the relevant place, - /// because it won't actually be tested. - place: Option>, - /// Testable condition to compare the place to (e.g. "is 3" or "is Some"). - /// - /// If `None`, this pattern node is irrefutable or an or-pattern, - /// though it might have refutable descendants. - testable_case: Option>, - - /// Immediate subpatterns of a node that is *not* an or-pattern. - subpats: Vec>, - /// Immediate subpatterns of an or-pattern node. - /// - /// Invariant: If this is Some, then fields `subpats`, `testable_case`, - /// and `binding` must all be empty. - or_subpats: Option]>>, + kind: InterPatKind<'tcx>, ascriptions: Vec>, - /// Binding to establish for a [`PatKind::Binding`] node. - binding: Option>, - /// Span field of the THIR pattern this node was created from. pattern_span: Span, /// True if this pattern can never match, because all of its alternatives @@ -221,6 +189,33 @@ struct InterPat<'tcx> { is_never: bool, } +enum InterPatKind<'tcx> { + Or { + /// The alternatives of an or-pattern, e.g. `P` and `Q` in `P | Q`. + or_subpats: Box<[InterPat<'tcx>]>, + }, + + /// Pattern node that performs some kind of test on a place. + Refutable { + /// Place that this pattern node will test. + place: Place<'tcx>, + /// Testable condition to compare the place to (e.g. "is 3" or "is Some"). + /// + /// Invariant: Must not be [`TestableCase::Or`]. + testable_case: TestableCase<'tcx>, + /// Immediate subpatterns. + subpats: Vec>, + }, + + /// Pattern node that doesn't test anything, though it might have refutable descendants. + Irrefutable { + /// Immediate subpatterns. + subpats: Vec>, + /// Binding to establish for a [`PatKind::Binding`] node. + binding: Option>, + }, +} + impl<'tcx> InterPat<'tcx> { fn lower_thir_pat( cx: &mut Builder<'_, 'tcx>, @@ -250,44 +245,49 @@ impl<'tcx> InterPat<'tcx> { } } - // Variables that will become `InterPat` fields: let place = place_builder.try_to_place(cx); - let mut subpats = vec![]; - let mut or_subpats = None; - let mut ascriptions = vec![]; - let mut binding = None; // Apply any type ascriptions to the value at `match_pair.place`. + let mut ascriptions = vec![]; if let Some(place) = place && let Some(extra) = &pattern.extra { - for &Ascription { ref annotation, variance } in &extra.ascriptions { - ascriptions.push(super::Ascription { + ascriptions.extend(extra.ascriptions.iter().map( + |&Ascription { ref annotation, variance }| super::Ascription { source: place, annotation: annotation.clone(), variance, - }); - } + }, + )); } - let testable_case = match pattern.kind { - PatKind::Missing | PatKind::Wild | PatKind::Error(_) => None, + // For refutable nodes a place must be available, either because it is not a + // closure upvar or because it was captured. + let unwrap_place = || place.expect("refutable patterns must have captured a place"); + + let kind: InterPatKind<'_> = match pattern.kind { + PatKind::Missing | PatKind::Wild | PatKind::Error(_) => { + InterPatKind::Irrefutable { subpats: vec![], binding: None } + } PatKind::Or { ref pats } => { - or_subpats = Some( - pats.iter() - .map(|subpat| InterPat::lower_thir_pat(cx, place_builder.clone(), subpat)) - .collect::>(), - ); - None + let or_subpats = pats + .iter() + .map(|subpat| InterPat::lower_thir_pat(cx, place_builder.clone(), subpat)) + .collect::>(); + InterPatKind::Or { or_subpats } } PatKind::Range(ref range) => { assert_eq!(pattern.ty, range.ty); if range.is_full_range(cx.tcx) == Some(true) { - None + InterPatKind::Irrefutable { subpats: vec![], binding: None } } else { - Some(TestableCase::Range(Arc::clone(range))) + InterPatKind::Refutable { + place: unwrap_place(), + testable_case: TestableCase::Range(Arc::clone(range)), + subpats: vec![], + } } } @@ -311,27 +311,30 @@ impl<'tcx> InterPat<'tcx> { // which could be split out into their own kinds. PatConstKind::Other }; - Some(TestableCase::Constant { value, kind: const_kind }) + + InterPatKind::Refutable { + place: unwrap_place(), + testable_case: TestableCase::Constant { value, kind: const_kind }, + subpats: vec![], + } } PatKind::Binding { mode, var, is_shorthand, ref subpattern, .. } => { // 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 - subpats.push(InterPat::lower_thir_pat(cx, place_builder, subpattern)); - } + // This is the `x @ P` case; have to keep matching against `P` now. + let subpat: Option> = subpattern + .as_deref() + .map(|subpattern| InterPat::lower_thir_pat(cx, place_builder, subpattern)); // Then push this binding, after any bindings in the subpattern. - if let Some(place) = place { - binding = Some(super::Binding { - span: pattern.span, - source: place, - var_id: var, - binding_mode: mode, - is_shorthand, - }); - } - None + let binding = place.map(|place| super::Binding { + span: pattern.span, + source: place, + var_id: var, + binding_mode: mode, + is_shorthand, + }); + InterPatKind::Irrefutable { subpats: Vec::from_iter(subpat), binding } } PatKind::Array { ref prefix, ref slice, ref suffix } => { @@ -343,6 +346,8 @@ impl<'tcx> InterPat<'tcx> { ty::Array(_, len) => len.try_to_target_usize(cx.tcx), _ => None, }; + + let mut subpats = vec![]; if let Some(array_len) = array_len { for (subplace, subpat) in prefix_slice_suffix(&place_builder, Some(array_len), prefix, slice, suffix) @@ -361,9 +366,10 @@ impl<'tcx> InterPat<'tcx> { ); } - None + InterPatKind::Irrefutable { subpats, binding: None } } PatKind::Slice { ref prefix, ref slice, ref suffix } => { + let mut subpats = vec![]; for (subplace, subpat) in prefix_slice_suffix(&place_builder, None, prefix, slice, suffix) { @@ -373,24 +379,26 @@ impl<'tcx> InterPat<'tcx> { if prefix.is_empty() && slice.is_some() && suffix.is_empty() { // A slice pattern shaped like `[..]` is irrefutable. // It can match a slice of any length, so no length test is needed. - None + InterPatKind::Irrefutable { subpats, binding: None } } else { // Any other shape of slice pattern requires a length test. // Slice patterns with a `..` subpattern require a minimum // length; those without `..` require an exact length. - Some(TestableCase::Slice { + let testable_case = TestableCase::Slice { len: u64::try_from(prefix.len() + suffix.len()).unwrap(), op: if slice.is_some() { SliceLenOp::GreaterOrEqual } else { SliceLenOp::Equal }, - }) + }; + InterPatKind::Refutable { place: unwrap_place(), testable_case, subpats } } } PatKind::Variant { adt_def, variant_index, args: _, ref subpatterns } => { let downcast_place = place_builder.downcast(adt_def, variant_index); // `(x as Variant)` + let mut subpats = vec![]; for &FieldPat { field, pattern: ref subpat } in subpatterns { let subplace = downcast_place.clone_project(PlaceElem::Field(field, subpat.ty)); subpats.push(InterPat::lower_thir_pat(cx, subplace, subpat)); @@ -401,18 +409,20 @@ impl<'tcx> InterPat<'tcx> { let refutable = adt_def.variants().len() > 1 || adt_def.is_variant_list_non_exhaustive(); if refutable { - Some(TestableCase::Variant { adt_def, variant_index }) + let testable_case = TestableCase::Variant { adt_def, variant_index }; + InterPatKind::Refutable { place: unwrap_place(), testable_case, subpats } } else { - None + InterPatKind::Irrefutable { subpats, binding: None } } } PatKind::Leaf { ref subpatterns } => { + let mut subpats = vec![]; for &FieldPat { field, pattern: ref subpat } in subpatterns { let subplace = place_builder.clone_project(PlaceElem::Field(field, subpat.ty)); subpats.push(InterPat::lower_thir_pat(cx, subplace, subpat)); } - None + InterPatKind::Irrefutable { subpats, binding: None } } PatKind::Deref { pin: Pinnedness::Pinned, ref subpattern } => { @@ -420,20 +430,20 @@ impl<'tcx> InterPat<'tcx> { Some(p_ty) if p_ty.is_ref() => p_ty, _ => span_bug!(pattern.span, "bad type for pinned deref: {:?}", pattern.ty), }; - subpats.push(InterPat::lower_thir_pat( + let subpat = InterPat::lower_thir_pat( cx, // Project into the `Pin(_)` struct, then deref the inner `&` or `&mut`. place_builder.field(FieldIdx::ZERO, pinned_ref_ty).deref(), subpattern, - )); + ); - None + InterPatKind::Irrefutable { subpats: vec![subpat], binding: None } } PatKind::Deref { pin: Pinnedness::Not, ref subpattern } | PatKind::DerefPattern { ref subpattern, borrow: DerefPatBorrowMode::Box } => { - subpats.push(InterPat::lower_thir_pat(cx, place_builder.deref(), subpattern)); - None + let subpat = InterPat::lower_thir_pat(cx, place_builder.deref(), subpattern); + InterPatKind::Irrefutable { subpats: vec![subpat], binding: None } } PatKind::DerefPattern { @@ -446,41 +456,39 @@ impl<'tcx> InterPat<'tcx> { Ty::new_ref(cx.tcx, cx.tcx.lifetimes.re_erased, subpattern.ty, mutability), pattern.span, ); - subpats.push(InterPat::lower_thir_pat( - cx, - PlaceBuilder::from(temp).deref(), - subpattern, - )); - Some(TestableCase::Deref { temp, mutability }) + let subpat = + InterPat::lower_thir_pat(cx, PlaceBuilder::from(temp).deref(), subpattern); + InterPatKind::Refutable { + place: unwrap_place(), + testable_case: TestableCase::Deref { temp, mutability }, + subpats: vec![subpat], + } } PatKind::Guard { .. } => { // FIXME(guard_patterns) - None + InterPatKind::Irrefutable { subpats: vec![], binding: None } } - PatKind::Never => Some(TestableCase::Never), + PatKind::Never => InterPatKind::Refutable { + place: unwrap_place(), + testable_case: TestableCase::Never, + subpats: vec![], + }, }; // A pattern node is guaranteed to never match if one of these is true: // - The node itself is a never pattern (`!`). // - It is not an or-pattern, and one of its subpatterns will never match. // - It is an or-pattern, and _all_ of its or-subpatterns will never match. - let is_never = matches!(pattern.kind, PatKind::Never) - || subpats.iter().any(|subpat| subpat.is_never) - || or_subpats - .as_ref() - .is_some_and(|or_subpats| or_subpats.iter().all(|subpat| subpat.is_never)); - - InterPat { - place, - testable_case, - subpats, - or_subpats, - ascriptions, - binding, - pattern_span: pattern.span, - is_never, - } + let is_never = match &kind { + InterPatKind::Refutable { testable_case: TestableCase::Never, .. } => true, + InterPatKind::Refutable { subpats, .. } | InterPatKind::Irrefutable { subpats, .. } => { + subpats.iter().any(|subpat| subpat.is_never) + } + InterPatKind::Or { or_subpats } => or_subpats.iter().all(|subpat| subpat.is_never), + }; + + InterPat { kind, ascriptions, pattern_span: pattern.span, is_never } } } From b12184e40190d7aa87279946ae6dae1290c31cfd Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 3 Aug 2026 00:03:21 +1000 Subject: [PATCH 2/2] Distinguish or/testable patterns in `MatchPairTree` --- .../src/builder/matches/buckets.rs | 30 ++++++--- .../src/builder/matches/match_pair.rs | 18 ++--- .../src/builder/matches/mod.rs | 65 ++++++++++--------- .../src/builder/matches/test.rs | 14 ++-- .../src/builder/matches/util.rs | 59 +++++++++-------- 5 files changed, 96 insertions(+), 90 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/buckets.rs b/compiler/rustc_mir_build/src/builder/matches/buckets.rs index 0d2e9bf87585d..36d3d78c21ec0 100644 --- a/compiler/rustc_mir_build/src/builder/matches/buckets.rs +++ b/compiler/rustc_mir_build/src/builder/matches/buckets.rs @@ -2,12 +2,12 @@ use std::cmp::Ordering; use rustc_data_structures::fx::FxIndexMap; use rustc_middle::mir::Place; -use rustc_middle::span_bug; +use rustc_middle::{bug, span_bug}; use tracing::debug; use crate::builder::Builder; use crate::builder::matches::{ - Candidate, PatConstKind, SliceLenOp, Test, TestBranch, TestKind, TestableCase, + Candidate, MatchPairKind, PatConstKind, SliceLenOp, Test, TestBranch, TestKind, TestableCase, }; /// Output of [`Builder::partition_candidates_into_buckets`]. @@ -131,17 +131,22 @@ 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_testable_case) = + candidate.match_pairs.iter().enumerate().find_map(|(i, mp)| { + if let MatchPairKind::Testable { place, ref testable_case, .. } = mp.kind + && place == test_place + { + Some((i, testable_case)) + } else { + None + } + })?; // 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_ // with its test branch, but still needs a more specific test. let fully_matched; - let ret = match (&test.kind, &match_pair.testable_case) { + let ret = match (&test.kind, match_pair_testable_case) { // If we are performing a variant switch, then this // informs variant patterns, but nothing else. ( @@ -174,7 +179,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }; let is_conflicting_candidate = |candidate: &&mut Candidate<'tcx>| { candidate.match_pairs.iter().any(|mp| { - mp.place == Some(test_place) && is_covering_range(&mp.testable_case) + matches!(mp.kind, MatchPairKind::Testable { place, ref testable_case, .. } + if place == test_place && is_covering_range(testable_case) + ) }) }; if prior_candidates @@ -364,7 +371,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if fully_matched { // Replace the match pair by its sub-pairs. let match_pair = candidate.match_pairs.remove(match_pair_index); - candidate.match_pairs.extend(match_pair.subpairs); + let MatchPairKind::Testable { subpairs, .. } = match_pair.kind else { + bug!("match pair must have been refutable"); + }; + candidate.match_pairs.extend(subpairs); // Move or-patterns to the end. candidate.sort_match_pairs(); } 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 2dcfbf3ca3098..7ad21b3272783 100644 --- a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs +++ b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs @@ -10,7 +10,7 @@ use rustc_span::Span; use crate::builder::Builder; use crate::builder::expr::as_place::{PlaceBase, PlaceBuilder}; use crate::builder::matches::{ - FlatPat, MatchPairTree, PatConstKind, PatternExtraData, SliceLenOp, TestableCase, + FlatPat, MatchPairKind, MatchPairTree, PatConstKind, PatternExtraData, SliceLenOp, TestableCase, }; /// For an array or slice pattern's subpatterns (prefix/slice/suffix), returns a list @@ -130,13 +130,8 @@ fn squash_inter_pat<'tcx>( extra_data.bindings.push(super::SubpatternBindings::FromOrPattern); } - match_pairs.push(MatchPairTree { - // Or-patterns never need a place during MIR building. - place: None, - testable_case: TestableCase::Or { pats: or_subpats }, - subpairs: vec![], - pattern_span, - }); + match_pairs + .push(MatchPairTree { kind: MatchPairKind::Or { or_subpats }, pattern_span }); } InterPatKind::Refutable { place, testable_case, subpats } => { @@ -148,11 +143,8 @@ fn squash_inter_pat<'tcx>( } // This pattern is refutable, so push a new match-pair node. - assert!(!matches!(testable_case, TestableCase::Or { .. })); match_pairs.push(MatchPairTree { - place: Some(place), - testable_case, - subpairs, + kind: MatchPairKind::Testable { place, testable_case, subpairs }, pattern_span, }); } @@ -200,8 +192,6 @@ enum InterPatKind<'tcx> { /// Place that this pattern node will test. place: Place<'tcx>, /// Testable condition to compare the place to (e.g. "is 3" or "is Some"). - /// - /// Invariant: Must not be [`TestableCase::Or`]. testable_case: TestableCase<'tcx>, /// Immediate subpatterns. subpats: Vec>, diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index 109f4de2698a4..ca1eebb3c69cf 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -1030,7 +1030,7 @@ struct Candidate<'tcx> { /// (see [`Builder::test_remaining_match_pairs_after_or`]). /// /// Invariants: - /// - All or-patterns ([`TestableCase::Or`]) have been sorted to the end. + /// - All or-patterns ([`MatchPairKind::Or`]) have been sorted to the end. match_pairs: Vec>, /// ...and if this is non-empty, one of these subcandidates also has to match... @@ -1116,14 +1116,14 @@ impl<'tcx> Candidate<'tcx> { /// 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.testable_case, TestableCase::Or { .. })); + self.match_pairs.sort_by_key(|pair| matches!(pair.kind, MatchPairKind::Or { .. })); } /// Returns whether the first match pair of this candidate is an or-pattern. fn starts_with_or_pattern(&self) -> bool { matches!( - &*self.match_pairs, - [MatchPairTree { testable_case: TestableCase::Or { .. }, .. }, ..] + self.match_pairs.first(), + Some(MatchPairTree { kind: MatchPairKind::Or { .. }, .. }) ) } @@ -1223,7 +1223,6 @@ enum TestableCase<'tcx> { Slice { len: u64, op: SliceLenOp }, Deref { temp: Place<'tcx>, mutability: Mutability }, Never, - Or { pats: Box<[FlatPat<'tcx>]> }, } impl<'tcx> TestableCase<'tcx> { @@ -1261,32 +1260,32 @@ enum PatConstKind { /// Each node also has a list of subpairs (possibly empty) that must also match, /// and some additional information from the THIR pattern it represents. #[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 `testable_case` is `Or`. - /// Therefore this must be `Some(_)` after or-pattern expansion. - place: Option>, - - /// ... must pass this test... - testable_case: TestableCase<'tcx>, - - /// ... and these subpairs must match. - /// - /// --- - /// Subpairs typically represent tests that can only be performed after their - /// parent has succeeded. For example, the pattern `Some(3)` might have an - /// outer match pair that tests for the variant `Some`, and then a subpair - /// that tests its field for the value `3`. - subpairs: Vec, +struct MatchPairTree<'tcx> { + kind: MatchPairKind<'tcx>, /// Span field of the THIR pattern this node was created from. pattern_span: Span, } +#[derive(Debug, Clone)] +enum MatchPairKind<'tcx> { + Or { + or_subpats: Box<[FlatPat<'tcx>]>, + }, + Testable { + /// Place that will be tested. + place: Place<'tcx>, + /// Test to perform against the place, and the desired outcome. + testable_case: TestableCase<'tcx>, + + /// Further tests that can only be performed after this test has succeeded. + /// For example, in the pattern `Some(3)` this node might represent a test + /// for the variant `Some`, while a subpair would test its field for the + /// value `3`. + subpairs: Vec>, + }, +} + /// A runtime test to perform to determine which candidates match a scrutinee place. /// /// The kind of test to perform is indicated by [`TestKind`]. @@ -1950,10 +1949,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate: &mut Candidate<'tcx>, match_pair: MatchPairTree<'tcx>, ) { - let TestableCase::Or { pats } = match_pair.testable_case else { bug!() }; - debug!("expanding or-pattern: candidate={:#?}\npats={:#?}", candidate, pats); + let MatchPairKind::Or { or_subpats } = match_pair.kind else { bug!() }; + debug!("expanding or-pattern: candidate={:#?}\nor_subpats={:#?}", candidate, or_subpats); candidate.or_span = Some(match_pair.pattern_span); - candidate.subcandidates = pats + candidate.subcandidates = or_subpats .into_iter() .map(|flat_pat| Candidate::from_flat_pat(flat_pat, candidate.has_guard)) .collect(); @@ -2118,7 +2117,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { debug_assert!( remaining_match_pairs .iter() - .all(|match_pair| matches!(match_pair.testable_case, TestableCase::Or { .. })) + .all(|match_pair| matches!(match_pair.kind, MatchPairKind::Or { .. })) ); // Visit each leaf candidate within this subtree, add a copy of the remaining @@ -2169,8 +2168,10 @@ 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(); + + let MatchPairKind::Testable { place: match_place, .. } = match_pair.kind else { + bug!("match pair must be testable") + }; debug!(?test, ?match_pair); (match_place, test) diff --git a/compiler/rustc_mir_build/src/builder/matches/test.rs b/compiler/rustc_mir_build/src/builder/matches/test.rs index 1c234bb8d70dc..8e8c73bcb87a2 100644 --- a/compiler/rustc_mir_build/src/builder/matches/test.rs +++ b/compiler/rustc_mir_build/src/builder/matches/test.rs @@ -19,7 +19,8 @@ use tracing::{debug, instrument}; use crate::builder::Builder; use crate::builder::matches::{ - MatchPairTree, PatConstKind, SliceLenOp, Test, TestBranch, TestKind, TestableCase, + MatchPairKind, MatchPairTree, PatConstKind, SliceLenOp, Test, TestBranch, TestKind, + TestableCase, }; impl<'a, 'tcx> Builder<'a, 'tcx> { @@ -30,7 +31,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &mut self, match_pair: &MatchPairTree<'tcx>, ) -> Test<'tcx> { - let kind = match match_pair.testable_case { + // Or-patterns are not tested directly; instead they are expanded into subcandidates, + // which are then distinguished by testing whatever non-or patterns they contain. + let MatchPairKind::Testable { ref testable_case, .. } = match_pair.kind else { + bug!("or-patterns should have already been handled") + }; + let kind = match *testable_case { TestableCase::Variant { adt_def, variant_index: _ } => TestKind::Switch { adt_def }, TestableCase::Constant { value: _, kind: PatConstKind::Bool } => TestKind::If, @@ -51,10 +57,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { TestableCase::Deref { temp, mutability } => TestKind::Deref { temp, mutability }, TestableCase::Never => TestKind::Never, - - // Or-patterns are not tested directly; instead they are expanded into subcandidates, - // which are then distinguished by testing whatever non-or patterns they contain. - TestableCase::Or { .. } => bug!("or-patterns should have already been handled"), }; Test { span: match_pair.pattern_span, kind } diff --git a/compiler/rustc_mir_build/src/builder/matches/util.rs b/compiler/rustc_mir_build/src/builder/matches/util.rs index 3246dab73dcbf..fa94a41bad339 100644 --- a/compiler/rustc_mir_build/src/builder/matches/util.rs +++ b/compiler/rustc_mir_build/src/builder/matches/util.rs @@ -6,7 +6,9 @@ use tracing::debug; use crate::builder::Builder; use crate::builder::expr::as_place::PlaceBase; -use crate::builder::matches::{Binding, Candidate, FlatPat, MatchPairTree, TestableCase}; +use crate::builder::matches::{ + Binding, Candidate, FlatPat, MatchPairKind, MatchPairTree, TestableCase, +}; impl<'a, 'tcx> Builder<'a, 'tcx> { /// Creates a false edge to `imaginary_target` and a real edge to @@ -159,35 +161,36 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { } fn visit_match_pair(&mut self, match_pair: &MatchPairTree<'tcx>) { - if let TestableCase::Or { pats, .. } = &match_pair.testable_case { - for flat_pat in pats.iter() { - self.visit_flat_pat(flat_pat) - } - } else if matches!(match_pair.testable_case, TestableCase::Deref { .. }) { - // The subpairs of a deref pattern are all places relative to the deref temporary, so we - // don't fake borrow them. Problem is, if we only shallowly fake-borrowed - // `match_pair.place`, this would allow: - // ``` - // let mut b = Box::new(false); - // match b { - // deref!(true) => {} // not reached because `*b == false` - // _ if { *b = true; false } => {} // not reached because the guard is `false` - // deref!(false) => {} // not reached because the guard changed it - // // UB because we reached the unreachable. - // } - // ``` - // Hence we fake borrow using a deep borrow. - if let Some(place) = match_pair.place { - self.fake_borrow(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); + match match_pair.kind { + MatchPairKind::Or { ref or_subpats } => { + for flat_pat in or_subpats { + self.visit_flat_pat(flat_pat); + } } + MatchPairKind::Testable { place, ref testable_case, ref subpairs } => { + if matches!(testable_case, TestableCase::Deref { .. }) { + // The subpairs of a deref pattern are all places relative to the deref temporary, so we + // don't fake borrow them. Problem is, if we only shallowly fake-borrowed + // `match_pair.place`, this would allow: + // ``` + // let mut b = Box::new(false); + // match b { + // deref!(true) => {} // not reached because `*b == false` + // _ if { *b = true; false } => {} // not reached because the guard is `false` + // deref!(false) => {} // not reached because the guard changed it + // // UB because we reached the unreachable. + // } + // ``` + // Hence we fake borrow using a deep borrow. + self.fake_borrow(place, FakeBorrowKind::Deep); + } else { + // Insert a Shallow borrow of any place that is switched on. + self.fake_borrow(place, FakeBorrowKind::Shallow); - for subpair in &match_pair.subpairs { - self.visit_match_pair(subpair); + for subpair in subpairs { + self.visit_match_pair(subpair); + } + } } } }