From df8d446cb7c93babd61f80530590c494e1f95dee Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 27 Jul 2024 19:15:07 +0200 Subject: [PATCH 1/8] Add tests --- .../mir-opt/building/match/exponential_or.rs | 12 -- ...exponential.SimplifyCfg-initial.after.mir} | 4 +- tests/mir-opt/building/match/or_patterns.rs | 22 ++++ ....simplification_subtleties.built.after.mir | 106 ++++++++++++++++++ tests/mir-opt/jump_threading.rs | 30 ++--- tests/mir-opt/unreachable_enum_branching.rs | 2 +- 6 files changed, 146 insertions(+), 30 deletions(-) delete mode 100644 tests/mir-opt/building/match/exponential_or.rs rename tests/mir-opt/building/match/{exponential_or.match_tuple.SimplifyCfg-initial.after.mir => or_patterns.exponential.SimplifyCfg-initial.after.mir} (93%) create mode 100644 tests/mir-opt/building/match/or_patterns.rs create mode 100644 tests/mir-opt/building/match/or_patterns.simplification_subtleties.built.after.mir diff --git a/tests/mir-opt/building/match/exponential_or.rs b/tests/mir-opt/building/match/exponential_or.rs deleted file mode 100644 index 89963b9bdf444..0000000000000 --- a/tests/mir-opt/building/match/exponential_or.rs +++ /dev/null @@ -1,12 +0,0 @@ -// skip-filecheck -// Test that simple or-patterns don't get expanded to exponentially large CFGs - -// EMIT_MIR exponential_or.match_tuple.SimplifyCfg-initial.after.mir -fn match_tuple(x: (u32, bool, Option, u32)) -> u32 { - match x { - (y @ (1 | 4), true | false, Some(1 | 8) | None, z @ (6..=9 | 13..=16)) => y ^ z, - _ => 0, - } -} - -fn main() {} diff --git a/tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/or_patterns.exponential.SimplifyCfg-initial.after.mir similarity index 93% rename from tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir rename to tests/mir-opt/building/match/or_patterns.exponential.SimplifyCfg-initial.after.mir index 596dcef85fd21..6ac452ba81a25 100644 --- a/tests/mir-opt/building/match/exponential_or.match_tuple.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/or_patterns.exponential.SimplifyCfg-initial.after.mir @@ -1,6 +1,6 @@ -// MIR for `match_tuple` after SimplifyCfg-initial +// MIR for `exponential` after SimplifyCfg-initial -fn match_tuple(_1: (u32, bool, Option, u32)) -> u32 { +fn exponential(_1: (u32, bool, Option, u32)) -> u32 { debug x => _1; let mut _0: u32; let mut _2: isize; diff --git a/tests/mir-opt/building/match/or_patterns.rs b/tests/mir-opt/building/match/or_patterns.rs new file mode 100644 index 0000000000000..29e373484418b --- /dev/null +++ b/tests/mir-opt/building/match/or_patterns.rs @@ -0,0 +1,22 @@ +// skip-filecheck + +// EMIT_MIR or_patterns.exponential.SimplifyCfg-initial.after.mir +fn exponential(x: (u32, bool, Option, u32)) -> u32 { + // Test that simple or-patterns don't get expanded to exponentially large CFGs + match x { + (y @ (1 | 4), true | false, Some(1 | 8) | None, z @ (6..=9 | 13..=16)) => y ^ z, + _ => 0, + } +} + +// EMIT_MIR or_patterns.simplification_subtleties.built.after.mir +fn simplification_subtleties() { + // Test that we don't naively sort the two `2`s together and confuse the failure paths. + match (1, true) { + (1 | 2, false | false) => unreachable!(), + (2, _) => unreachable!(), + _ => {} + } +} + +fn main() {} diff --git a/tests/mir-opt/building/match/or_patterns.simplification_subtleties.built.after.mir b/tests/mir-opt/building/match/or_patterns.simplification_subtleties.built.after.mir new file mode 100644 index 0000000000000..8ce0217053531 --- /dev/null +++ b/tests/mir-opt/building/match/or_patterns.simplification_subtleties.built.after.mir @@ -0,0 +1,106 @@ +// MIR for `simplification_subtleties` after built + +fn simplification_subtleties() -> () { + let mut _0: (); + let mut _1: (i32, bool); + let mut _2: !; + let mut _3: !; + + bb0: { + StorageLive(_1); + _1 = (const 1_i32, const true); + PlaceMention(_1); + switchInt((_1.0: i32)) -> [1: bb2, 2: bb4, otherwise: bb1]; + } + + bb1: { + switchInt((_1.0: i32)) -> [2: bb13, otherwise: bb12]; + } + + bb2: { + goto -> bb6; + } + + bb3: { + goto -> bb1; + } + + bb4: { + goto -> bb6; + } + + bb5: { + goto -> bb1; + } + + bb6: { + switchInt((_1.1: bool)) -> [0: bb8, otherwise: bb7]; + } + + bb7: { + goto -> bb5; + } + + bb8: { + goto -> bb11; + } + + bb9: { + goto -> bb11; + } + + bb10: { + goto -> bb7; + } + + bb11: { + falseEdge -> [real: bb17, imaginary: bb1]; + } + + bb12: { + _0 = const (); + goto -> bb20; + } + + bb13: { + falseEdge -> [real: bb16, imaginary: bb12]; + } + + bb14: { + goto -> bb12; + } + + bb15: { + FakeRead(ForMatchedPlace(None), _1); + unreachable; + } + + bb16: { + StorageLive(_3); + _3 = core::panicking::panic(const "internal error: entered unreachable code") -> bb21; + } + + bb17: { + StorageLive(_2); + _2 = core::panicking::panic(const "internal error: entered unreachable code") -> bb21; + } + + bb18: { + StorageDead(_2); + goto -> bb20; + } + + bb19: { + StorageDead(_3); + goto -> bb20; + } + + bb20: { + StorageDead(_1); + return; + } + + bb21 (cleanup): { + resume; + } +} diff --git a/tests/mir-opt/jump_threading.rs b/tests/mir-opt/jump_threading.rs index 6486a321e693e..5f459c5b252bb 100644 --- a/tests/mir-opt/jump_threading.rs +++ b/tests/mir-opt/jump_threading.rs @@ -12,29 +12,29 @@ use std::ops::ControlFlow; fn too_complex(x: Result) -> Option { // CHECK-LABEL: fn too_complex( // CHECK: bb0: { - // CHECK: switchInt(move {{_.*}}) -> [0: bb3, 1: bb2, otherwise: bb1]; - // CHECK: bb1: { + // CHECK: switchInt(move {{_.*}}) -> [0: [[continue1:bb.*]], 1: [[break1:bb.*]], otherwise: [[unreachable:bb.*]]]; + // CHECK: [[unreachable]]: { // CHECK: unreachable; - // CHECK: bb2: { + // CHECK: [[break1]]: { // CHECK: [[controlflow:_.*]] = ControlFlow::::Break( - // CHECK: goto -> bb8; - // CHECK: bb3: { + // CHECK: goto -> [[break2:bb.*]]; + // CHECK: [[continue1]]: { // CHECK: [[controlflow]] = ControlFlow::::Continue( - // CHECK: goto -> bb4; - // CHECK: bb4: { - // CHECK: goto -> bb6; - // CHECK: bb5: { + // CHECK: goto -> [[continue2:bb.*]]; + // CHECK: [[continue2]]: { + // CHECK: goto -> [[continue3:bb.*]]; + // CHECK: [[break3:bb.*]]: { // CHECK: {{_.*}} = (([[controlflow]] as Break).0: usize); // CHECK: _0 = Option::::None; - // CHECK: goto -> bb7; - // CHECK: bb6: { + // CHECK: goto -> [[return:bb.*]]; + // CHECK: [[continue3]]: { // CHECK: {{_.*}} = (([[controlflow]] as Continue).0: i32); // CHECK: _0 = Option::::Some( - // CHECK: goto -> bb7; - // CHECK: bb7: { + // CHECK: goto -> [[return]]; + // CHECK: [[return]]: { // CHECK: return; - // CHECK: bb8: { - // CHECK: goto -> bb5; + // CHECK: [[break2]]: { + // CHECK: goto -> [[break3]]; match { match x { Ok(v) => ControlFlow::Continue(v), diff --git a/tests/mir-opt/unreachable_enum_branching.rs b/tests/mir-opt/unreachable_enum_branching.rs index fac14042b1075..e272ea9b7226d 100644 --- a/tests/mir-opt/unreachable_enum_branching.rs +++ b/tests/mir-opt/unreachable_enum_branching.rs @@ -49,7 +49,7 @@ struct Plop { fn simple() { // CHECK-LABEL: fn simple( // CHECK: [[discr:_.*]] = discriminant( - // CHECK: switchInt(move [[discr]]) -> [0: [[unreachable:bb.*]], 1: [[unreachable]], 2: bb2, otherwise: [[unreachable]]]; + // CHECK: switchInt(move [[discr]]) -> [0: [[unreachable:bb.*]], 1: [[unreachable]], 2: {{bb.*}}, otherwise: [[unreachable]]]; // CHECK: [[unreachable]]: { // CHECK-NEXT: unreachable; match Test1::C { From 89bd211c98f28d2b4a0fab93e004affdc968c247 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 27 Jul 2024 20:45:22 +0200 Subject: [PATCH 2/8] Explain or-pattern shortcutting a bit better --- .../rustc_mir_build/src/build/matches/mod.rs | 69 ++++++++++++++----- 1 file changed, 50 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index cae4aa7bad3d2..d518e7cd88d89 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1892,8 +1892,31 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// exponentially large CFGs in cases like `(1 | 2, 3 | 4, ...)`. The candidate should have been /// expanded with `create_or_subcandidates`. /// - /// Given a pattern `(P | Q, R | S)` we (in principle) generate a CFG like - /// so: + /// Given a pattern `(P | Q, R | S)` we would like to generate a CFG like so: + /// + /// ```text + /// ... + /// +---------------+------------+ + /// | | | + /// [ P matches ] [ Q matches ] [ otherwise ] + /// | | | + /// +---------------+ | + /// | ... + /// [ match R, S ] + /// | + /// ... + /// ``` + /// + /// In practice there are some complications: + /// + /// * If `P` or `Q` has bindings or type ascriptions, they can refer to different places in each + /// branch so we must generate separate branches for each case. + /// * If there's a guard, we must also keep branches separate as this changes how many times + /// the guard is run. + /// * If `P` succeeds and `R | S` fails, we know `(Q, R | S)` will fail too. So we could skip + /// testing of `Q` in that case. Because we can't distinguish pattern failure from guard + /// failure, we only do this optimization when there is no guard. We then get this (note how + /// we don't test `Q` if `(P, R | S)` fails): /// /// ```text /// [ start ] @@ -1921,26 +1944,34 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// [ Success ] [ Failure ] /// ``` /// - /// In practice there are some complications: - /// - /// * If there's a guard, then the otherwise branch of the first match on - /// `R | S` goes to a test for whether `Q` matches, and the control flow - /// doesn't merge into a single success block until after the guard is - /// tested. - /// * If neither `P` or `Q` has any bindings or type ascriptions and there - /// isn't a match guard, then we create a smaller CFG like: + /// * If there's a guard, then the otherwise branch of the first match on `R | S` goes to a test + /// for whether `Q` matches, and the control flow doesn't merge into a single success block + /// until after the guard is tested. In other words, the branches are kept entirely separate: /// /// ```text - /// ... - /// +---------------+------------+ - /// | | | - /// [ P matches ] [ Q matches ] [ otherwise ] - /// | | | - /// +---------------+ | - /// | ... - /// [ match R, S ] + /// [ start ] /// | - /// ... + /// [ match P, Q ] + /// | + /// +---------------------------+-------------+------------------------------------+ + /// | | | | + /// V | V V + /// [ P matches ] | [ Q matches ] [ otherwise ] + /// | | | | + /// V [ otherwise ] V | + /// [ match R, S ] ^ [ match R, S ] | + /// | | | | + /// +--------------+------------+ +--------------+------------+ | + /// | | | | | | + /// V V V V V | + /// [ R matches ] [ S matches ] [ R matches ] [ S matches ] [otherwise ] | + /// | | | | | | + /// +--------------+--------------------------+--------------+ | | + /// | | | + /// | +--------+ + /// | | + /// V V + /// [ Success ] [ Failure ] /// ``` /// /// Note that this takes place _after_ the subcandidates have participated From e237f0203965b73dd895eb33373d9cf2258a95b6 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 28 Jul 2024 10:38:32 +0200 Subject: [PATCH 3/8] Pick the or-pattern failure block earlier `match_candidates` can change the `otherwise_block`, so the previous code was morally incorrect (if not in practice, for some reason). --- compiler/rustc_mir_build/src/build/matches/mod.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index d518e7cd88d89..ace041c0c0c1f 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -2083,19 +2083,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { assert!(leaf_candidate.match_pairs.is_empty()); leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned()); + // The candidate was processed, and matching of the processed match pairs branched to + // its `pre_binding_block`. We therefore start from that to match the new match pairs. let or_start = leaf_candidate.pre_binding_block.unwrap(); - let otherwise = - self.match_candidates(span, scrutinee_span, or_start, &mut [leaf_candidate]); - // In a case like `(P | Q, R | S)`, if `P` succeeds and `R | S` fails, we know `(Q, - // R | S)` will fail too. If there is no guard, we skip testing of `Q` by branching - // directly to `last_otherwise`. If there is a guard, - // `leaf_candidate.otherwise_block` can be reached by guard failure as well, so we - // can't skip `Q`. + // In a case like `(P | Q, R | S)`, if `P` succeeds and `R | S` fails, we know `(Q, R | + // S)` will fail too. If there is no guard, we skip testing of `Q` by branching directly + // to `last_otherwise`. If there is a guard, `leaf_candidate.otherwise_block` can be + // reached by guard failure as well, so we can't skip `Q`. let or_otherwise = if leaf_candidate.has_guard { leaf_candidate.otherwise_block.unwrap() } else { last_otherwise.unwrap() }; + let otherwise = + self.match_candidates(span, scrutinee_span, or_start, &mut [leaf_candidate]); self.cfg.goto(otherwise, source_info, or_otherwise); }); } From e3620af37d243888a5a20a1c4539f22f5d9dd8db Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 27 Jul 2024 20:55:42 +0200 Subject: [PATCH 4/8] Precompute which or-patterns can be simplified --- .../src/build/matches/match_pair.rs | 15 +++++-- .../rustc_mir_build/src/build/matches/mod.rs | 43 +++++++++++++++---- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/match_pair.rs b/compiler/rustc_mir_build/src/build/matches/match_pair.rs index 95fec154918a5..79210277e2c9c 100644 --- a/compiler/rustc_mir_build/src/build/matches/match_pair.rs +++ b/compiler/rustc_mir_build/src/build/matches/match_pair.rs @@ -112,9 +112,12 @@ impl<'pat, 'tcx> MatchPairTree<'pat, 'tcx> { let test_case = match pattern.kind { PatKind::Wild | PatKind::Error(_) => default_irrefutable(), - PatKind::Or { ref pats } => TestCase::Or { - pats: pats.iter().map(|pat| FlatPat::new(place_builder.clone(), pat, cx)).collect(), - }, + PatKind::Or { ref pats } => { + let pats: Box<[_]> = + pats.iter().map(|pat| FlatPat::new(place_builder.clone(), pat, cx)).collect(); + let contains_bindings = pats.iter().any(|fpat| fpat.contains_bindings); + TestCase::Or { pats, contains_bindings } + } PatKind::Range(ref range) => { if range.is_full_range(cx.tcx) == Some(true) { @@ -255,4 +258,10 @@ impl<'pat, 'tcx> MatchPairTree<'pat, 'tcx> { MatchPairTree { place, test_case, subpairs, pattern } } + + /// Whether this recursively contains any bindings or ascriptions. + pub(super) fn contains_bindings(&self) -> bool { + matches!(self.test_case, TestCase::Or { contains_bindings: true, .. }) + || self.subpairs.iter().any(|p| p.contains_bindings()) + } } diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index ace041c0c0c1f..93a15b6e91ce3 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -997,11 +997,15 @@ impl<'tcx> PatternExtraData<'tcx> { /// Will typically be incorporated into a [`Candidate`]. #[derive(Debug, Clone)] struct FlatPat<'pat, 'tcx> { - /// To match the pattern, all of these must be satisfied... + /// 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>, + /// Whether this recursively (i.e. including inside sub-or-patterns) contains bindings or + /// ascriptions. + contains_bindings: bool, + extra_data: PatternExtraData<'tcx>, } @@ -1025,7 +1029,10 @@ impl<'tcx, 'pat> FlatPat<'pat, 'tcx> { // bindings/ascriptions, and sort or-patterns after other match pairs. cx.simplify_match_pairs(&mut match_pairs, &mut extra_data); - Self { match_pairs, extra_data } + let contains_bindings = + !extra_data.is_empty() || match_pairs.iter().any(|mp| mp.contains_bindings()); + + Self { match_pairs, extra_data, contains_bindings } } } @@ -1242,14 +1249,32 @@ struct Ascription<'tcx> { /// - See [`Builder::expand_and_match_or_candidates`]. #[derive(Debug, Clone)] enum TestCase<'pat, 'tcx> { - Irrefutable { binding: Option>, ascription: Option> }, - Variant { adt_def: ty::AdtDef<'tcx>, variant_index: VariantIdx }, - Constant { value: mir::Const<'tcx> }, + Irrefutable { + binding: Option>, + ascription: Option>, + }, + Variant { + adt_def: ty::AdtDef<'tcx>, + variant_index: VariantIdx, + }, + Constant { + value: mir::Const<'tcx>, + }, Range(&'pat PatRange<'tcx>), - Slice { len: usize, variable_length: bool }, - Deref { temp: Place<'tcx>, mutability: Mutability }, + Slice { + len: usize, + variable_length: bool, + }, + Deref { + temp: Place<'tcx>, + mutability: Mutability, + }, Never, - Or { pats: Box<[FlatPat<'pat, 'tcx>]> }, + Or { + pats: Box<[FlatPat<'pat, 'tcx>]>, + /// Whether this recursively contains any bindings or ascriptions. + contains_bindings: bool, + }, } impl<'pat, 'tcx> TestCase<'pat, 'tcx> { @@ -1877,7 +1902,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate: &mut Candidate<'pat, 'tcx>, match_pair: MatchPairTree<'pat, 'tcx>, ) { - let TestCase::Or { pats } = match_pair.test_case else { bug!() }; + let TestCase::Or { pats, .. } = match_pair.test_case else { bug!() }; debug!("expanding or-pattern: candidate={:#?}\npats={:#?}", candidate, pats); candidate.or_span = Some(match_pair.pattern.span); candidate.subcandidates = pats From 3193c25d78f0f85383c6ebca2e0919df16004770 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 28 Jul 2024 15:22:54 +0200 Subject: [PATCH 5/8] Reify the steps of `Candidate` processing into an enum --- .../rustc_mir_build/src/build/matches/mod.rs | 264 +++++++++++------- 1 file changed, 167 insertions(+), 97 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 93a15b6e91ce3..ca2551c98a781 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1036,6 +1036,41 @@ impl<'tcx, 'pat> FlatPat<'pat, 'tcx> { } } +#[derive(Debug)] +enum CandidateState<'pat, 'tcx> { + /// Still some match pairs to process. + Incomplete, + /// This candidate has been fully processed. + Complete { + /// When the scrutinee matches, we branch to this block. + /// + /// After the match tree has been lowered, [`Builder::lower_match_arms`] will use this as + /// the start point for lowering bindings and guards, and then jump to a shared block + /// containing the arm body. + success_block: BasicBlock, + /// If further conditions are checked after the `success_block` (because of a guard or + /// nested or-patterns), and these conditions fail, branch to `continue_matching_block` to + /// continue matching subsequent candidates. + continue_matching_block: BasicBlock, + }, + /// This candidate had an or-pattern and got expanded into subcandidates. + Expanded { + /// The span of the or-pattern they came from. + or_span: Span, + /// When processing an or-pattern, we construct new `Candidate`s for each alternative. We + /// store them here for two reasons: + /// - because we need them allocated somewhere to borrow mutably from them; + /// - to serve as _output_ of match tree lowering, allowing later steps to see the leaf + /// candidates that represent a match of the entire match arm. + /// + /// Invariant: at the end of match tree lowering, this must not contain an + /// `is_never` candidate, because that would break binding consistency. + /// - See [`Builder::remove_never_subcandidates`]. + /// Invariant: this must not be empty. + subcandidates: Vec>, + }, +} + /// Candidates are a generalization of (a) top-level match arms, and /// (b) sub-branches of or-patterns, allowing the match-lowering process to handle /// them both in a mostly-uniform way. For example, the list of candidates passed @@ -1048,7 +1083,7 @@ impl<'tcx, 'pat> FlatPat<'pat, 'tcx> { /// the arm pattern to successfully match. #[derive(Debug)] struct Candidate<'pat, 'tcx> { - /// For the candidate to match, all of these must be satisfied... + /// For the candidate to match, all of these must be satisfied. /// /// --- /// Initially contains a list of match pairs created by [`FlatPat`], but is @@ -1071,24 +1106,13 @@ struct Candidate<'pat, 'tcx> { /// - All or-patterns ([`TestCase::Or`]) have been sorted to the end. match_pairs: Vec>, - /// ...and if this is non-empty, one of these subcandidates also has to match... - /// - /// --- - /// Initially a candidate has no subcandidates; they are added (and then immediately - /// lowered) during or-pattern expansion. Their main function is to serve as _output_ - /// of match tree lowering, allowing later steps to see the leaf candidates that - /// represent a match of the entire match arm. - /// - /// A candidate no subcandidates is either incomplete (if it has match pairs left), - /// or is a leaf in the match tree. A candidate with one or more subcandidates is - /// an internal node in the match tree. - /// - /// Invariant: at the end of match tree lowering, this must not contain an - /// `is_never` candidate, because that would break binding consistency. - /// - See [`Builder::remove_never_subcandidates`]. - subcandidates: Vec>, + /// A `Candidate` goes through several states during the algorithm. See `CandidateState` for + /// details. + state: CandidateState<'pat, 'tcx>, - /// ...and if there is a guard it must be evaluated; if it's `false` then branch to `otherwise_block`. + /// Whether this candidate is under a guard. This affects how we lower or-patterns, because the + /// guard must be run once for each possible expansion of the or-patterns in the match arm. When + /// this is `false` we can take shortcuts that reduce code duplication. /// /// --- /// For subcandidates, this is copied from the parent candidate, so it indicates @@ -1099,23 +1123,6 @@ struct Candidate<'pat, 'tcx> { /// ascriptions that must be established if this candidate succeeds. extra_data: PatternExtraData<'tcx>, - /// When setting `self.subcandidates`, we store here the span of the or-pattern they came from. - /// - /// --- - /// Invariant: it is `None` iff `subcandidates.is_empty()`. - /// - FIXME: We sometimes don't unset this when clearing `subcandidates`. - or_span: Option, - - /// The block before the `bindings` have been established. - /// - /// After the match tree has been lowered, [`Builder::lower_match_arms`] - /// will use this as the start point for lowering bindings and guards, and - /// then jump to a shared block containing the arm body. - pre_binding_block: Option, - - /// The block to branch to if the guard or a nested candidate fails to match. - otherwise_block: Option, - /// The earliest block that has only candidates >= this one as descendents. Used for false /// edges, see the doc for [`Builder::match_expr`]. false_edge_start_block: Option, @@ -1140,16 +1147,17 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> { fn from_flat_pat(flat_pat: FlatPat<'pat, 'tcx>, has_guard: bool) -> Self { Candidate { match_pairs: flat_pat.match_pairs, + state: CandidateState::Incomplete, extra_data: flat_pat.extra_data, has_guard, - subcandidates: Vec::new(), - or_span: None, - otherwise_block: None, - pre_binding_block: None, false_edge_start_block: None, } } + fn subcandidates_mut(&mut self) -> impl Iterator + DoubleEndedIterator { + self.state.subcandidates_mut().into_iter().flatten() + } + /// 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 { test_case: TestCase::Or { .. }, .. }, ..]) @@ -1162,7 +1170,7 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> { self, &mut (), &mut move |c, _| visit_leaf(c), - move |c, _| c.subcandidates.iter_mut(), + move |c, _| c.subcandidates_mut(), |_| {}, ); } @@ -1173,12 +1181,28 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> { self, &mut (), &mut move |c, _| visit_leaf(c), - move |c, _| c.subcandidates.iter_mut().rev(), + move |c, _| c.subcandidates_mut().rev(), |_| {}, ); } } +impl<'pat, 'tcx> CandidateState<'pat, 'tcx> { + fn into_subcandidates(self) -> Option>> { + match self { + CandidateState::Incomplete | CandidateState::Complete { .. } => None, + CandidateState::Expanded { subcandidates, .. } => Some(subcandidates), + } + } + + fn subcandidates_mut(&mut self) -> Option<&mut [Candidate<'pat, 'tcx>]> { + match self { + CandidateState::Incomplete | CandidateState::Complete { .. } => None, + CandidateState::Expanded { subcandidates, .. } => Some(subcandidates.as_mut_slice()), + } + } +} + /// A depth-first traversal of the `Candidate` and all of its recursive /// subcandidates. /// @@ -1201,13 +1225,16 @@ fn traverse_candidate<'pat, 'tcx: 'pat, C, T, I>( C: Borrow>, // Typically `Candidate` or `&mut Candidate` I: Iterator, { - if candidate.borrow().subcandidates.is_empty() { - visit_leaf(candidate, context) - } else { - for child in get_children(candidate, context) { - traverse_candidate(child, context, visit_leaf, get_children, complete_children); + match candidate.borrow().state { + CandidateState::Incomplete | CandidateState::Complete { .. } => { + visit_leaf(candidate, context) + } + CandidateState::Expanded { .. } => { + for child in get_children(candidate, context) { + traverse_candidate(child, context, visit_leaf, get_children, complete_children); + } + complete_children(context) } - complete_children(context) } } @@ -1459,10 +1486,14 @@ impl<'tcx> MatchTreeSubBranch<'tcx> { parent_data: &Vec>, ) -> Self { debug_assert!(candidate.match_pairs.is_empty()); + let CandidateState::Complete { success_block, continue_matching_block } = candidate.state + else { + bug!("a leaf candidate should be `Complete` at the end of the algorithm") + }; MatchTreeSubBranch { span: candidate.extra_data.span, - success_block: candidate.pre_binding_block.unwrap(), - otherwise_block: candidate.otherwise_block.unwrap(), + success_block, + otherwise_block: continue_matching_block, bindings: parent_data .iter() .flat_map(|d| &d.bindings) @@ -1491,7 +1522,7 @@ impl<'tcx> MatchTreeBranch<'tcx> { }, |inner_candidate, parent_data| { parent_data.push(inner_candidate.extra_data); - inner_candidate.subcandidates.into_iter() + inner_candidate.state.into_subcandidates().into_iter().flatten() }, |parent_data| { parent_data.pop(); @@ -1563,9 +1594,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let has_guard = candidate.has_guard; candidate.visit_leaves_rev(|leaf_candidate| { if let Some(next_candidate_start_block) = next_candidate_start_block { + let CandidateState::Complete { success_block, continue_matching_block } = + &mut leaf_candidate.state + else { + bug!() + }; let source_info = self.source_info(leaf_candidate.extra_data.span); // Falsely branch to `next_candidate_start_block` before reaching pre_binding. - let old_pre_binding = leaf_candidate.pre_binding_block.unwrap(); + let old_pre_binding = *success_block; let new_pre_binding = self.cfg.start_new_block(); self.false_edges( old_pre_binding, @@ -1573,18 +1609,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { next_candidate_start_block, source_info, ); - leaf_candidate.pre_binding_block = Some(new_pre_binding); + *success_block = new_pre_binding; if has_guard { // Falsely branch to `next_candidate_start_block` also if the guard fails. let new_otherwise = self.cfg.start_new_block(); - let old_otherwise = leaf_candidate.otherwise_block.unwrap(); + let old_otherwise = *continue_matching_block; self.false_edges( new_otherwise, old_otherwise, next_candidate_start_block, source_info, ); - leaf_candidate.otherwise_block = Some(new_otherwise); + *continue_matching_block = new_otherwise; } } assert!(leaf_candidate.false_edge_start_block.is_some()); @@ -1782,15 +1818,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate: &mut Candidate<'_, 'tcx>, start_block: BasicBlock, ) -> BasicBlock { - assert!(candidate.otherwise_block.is_none()); - assert!(candidate.pre_binding_block.is_none()); - assert!(candidate.subcandidates.is_empty()); + assert!(matches!(candidate.state, CandidateState::Incomplete)); - candidate.pre_binding_block = Some(start_block); - let otherwise_block = self.cfg.start_new_block(); // Create the otherwise block for this candidate, which is the // pre-binding block for the next candidate. - candidate.otherwise_block = Some(otherwise_block); + let otherwise_block = self.cfg.start_new_block(); + candidate.state = CandidateState::Complete { + success_block: start_block, + continue_matching_block: otherwise_block, + }; otherwise_block } @@ -1850,7 +1886,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Expand the or-pattern into subcandidates. self.create_or_subcandidates(candidate, or_match_pair); // Collect the newly created subcandidates. - for subcandidate in candidate.subcandidates.iter_mut() { + for subcandidate in candidate.subcandidates_mut() { expanded_candidates.push(subcandidate); } // Note that the subcandidates have been added to `expanded_candidates`, @@ -1879,10 +1915,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { all_except_last.all(|candidate| candidate.match_pairs.is_empty()) }); for candidate in candidates_to_expand.iter_mut() { - if !candidate.subcandidates.is_empty() { - self.merge_trivial_subcandidates(candidate); - self.remove_never_subcandidates(candidate); - } + self.merge_trivial_subcandidates(candidate); + self.remove_never_subcandidates(candidate); } // It's important to perform the above simplifications _before_ dealing // with remaining match pairs, to avoid exponential blowup if possible @@ -1902,15 +1936,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate: &mut Candidate<'pat, 'tcx>, match_pair: MatchPairTree<'pat, 'tcx>, ) { + assert!(matches!(candidate.state, CandidateState::Incomplete)); let TestCase::Or { pats, .. } = match_pair.test_case else { bug!() }; debug!("expanding or-pattern: candidate={:#?}\npats={:#?}", candidate, pats); - candidate.or_span = Some(match_pair.pattern.span); - candidate.subcandidates = pats + let or_span = match_pair.pattern.span; + let mut subcandidates: Vec<_> = pats .into_vec() .into_iter() .map(|flat_pat| Candidate::from_flat_pat(flat_pat, candidate.has_guard)) .collect(); - candidate.subcandidates[0].false_edge_start_block = candidate.false_edge_start_block; + subcandidates[0].false_edge_start_block = candidate.false_edge_start_block; + candidate.state = CandidateState::Expanded { or_span, subcandidates }; } /// Try to merge all of the subcandidates of the given candidate into one. This avoids @@ -2002,15 +2038,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// Note that this takes place _after_ the subcandidates have participated /// in match tree lowering. fn merge_trivial_subcandidates(&mut self, candidate: &mut Candidate<'_, 'tcx>) { - assert!(!candidate.subcandidates.is_empty()); + let CandidateState::Expanded { subcandidates, or_span } = &mut candidate.state else { + return; + }; if candidate.has_guard { // FIXME(or_patterns; matthewjasper) Don't give up if we have a guard. return; } // FIXME(or_patterns; matthewjasper) Try to be more aggressive here. - let can_merge = candidate.subcandidates.iter().all(|subcandidate| { - subcandidate.subcandidates.is_empty() && subcandidate.extra_data.is_empty() + let can_merge = subcandidates.iter().all(|subcandidate| { + !matches!(subcandidate.state, CandidateState::Expanded { .. }) + && subcandidate.extra_data.is_empty() }); if !can_merge { return; @@ -2018,12 +2057,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let mut last_otherwise = None; let shared_pre_binding_block = self.cfg.start_new_block(); - // This candidate is about to become a leaf, so unset `or_span`. - let or_span = candidate.or_span.take().unwrap(); - let source_info = self.source_info(or_span); + let source_info = self.source_info(*or_span); if candidate.false_edge_start_block.is_none() { - candidate.false_edge_start_block = candidate.subcandidates[0].false_edge_start_block; + candidate.false_edge_start_block = subcandidates[0].false_edge_start_block; } // Remove the (known-trivial) subcandidates from the candidate tree, @@ -2031,40 +2068,58 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // all to join up at a single shared pre-binding block. // (Note that the subcandidates have already had their part of the match // tree lowered by this point, which is why we can add a goto to them.) - for subcandidate in mem::take(&mut candidate.subcandidates) { - let subcandidate_block = subcandidate.pre_binding_block.unwrap(); - self.cfg.goto(subcandidate_block, source_info, shared_pre_binding_block); - last_otherwise = subcandidate.otherwise_block; + for subcandidate in mem::take(subcandidates) { + let CandidateState::Complete { success_block, continue_matching_block } = + subcandidate.state + else { + bug!() + }; + self.cfg.goto(success_block, source_info, shared_pre_binding_block); + last_otherwise = Some(continue_matching_block); } - candidate.pre_binding_block = Some(shared_pre_binding_block); - assert!(last_otherwise.is_some()); - candidate.otherwise_block = last_otherwise; + // This is a lie, some match pairs may remain. They'll be processed right away by + // `test_remaining_match_pairs_after_or` + candidate.state = CandidateState::Complete { + success_block: shared_pre_binding_block, + continue_matching_block: last_otherwise.unwrap(), + }; } /// Never subcandidates may have a set of bindings inconsistent with their siblings, /// which would break later code. So we filter them out. Note that we can't filter out /// top-level candidates this way. fn remove_never_subcandidates(&mut self, candidate: &mut Candidate<'_, 'tcx>) { - if candidate.subcandidates.is_empty() { - return; - } + let CandidateState::Expanded { subcandidates, .. } = &mut candidate.state else { return }; - candidate.subcandidates.retain_mut(|candidate| { + let mut last_otherwise = None; + subcandidates.retain_mut(|candidate| { if candidate.extra_data.is_never { candidate.visit_leaves(|subcandidate| { - let block = subcandidate.pre_binding_block.unwrap(); - // That block is already unreachable but needs a terminator to make the MIR well-formed. - let source_info = self.source_info(subcandidate.extra_data.span); - self.cfg.terminate(block, source_info, TerminatorKind::Unreachable); + match &subcandidate.state { + CandidateState::Incomplete | CandidateState::Expanded { .. } => bug!(), + CandidateState::Complete { success_block, continue_matching_block } => { + last_otherwise = Some(*continue_matching_block); + // That block is already unreachable but needs a terminator to make the MIR well-formed. + let source_info = self.source_info(subcandidate.extra_data.span); + self.cfg.terminate( + *success_block, + source_info, + TerminatorKind::Unreachable, + ); + } + } }); false } else { true } }); - if candidate.subcandidates.is_empty() { - // If `candidate` has become a leaf candidate, ensure it has a `pre_binding_block`. - candidate.pre_binding_block = Some(self.cfg.start_new_block()); + if subcandidates.is_empty() { + // If `candidate` has become a leaf candidate, update its state. + candidate.state = CandidateState::Complete { + success_block: self.cfg.start_new_block(), + continue_matching_block: last_otherwise.unwrap(), + }; } } @@ -2082,11 +2137,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { return; } - let or_span = candidate.or_span.unwrap_or(candidate.extra_data.span); + let or_span = match candidate.state { + CandidateState::Expanded { or_span, .. } => or_span, + // This branch is possible if we just merged trivial patterns. + _ => candidate.extra_data.span, + }; let source_info = self.source_info(or_span); + let mut last_otherwise = None; candidate.visit_leaves(|leaf_candidate| { - last_otherwise = leaf_candidate.otherwise_block; + let CandidateState::Complete { continue_matching_block, .. } = leaf_candidate.state + else { + bug!() + }; + last_otherwise = Some(continue_matching_block); }); let remaining_match_pairs = mem::take(&mut candidate.match_pairs); @@ -2102,6 +2166,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // match pairs to it, and then recursively lower the rest of the match tree // from that point. candidate.visit_leaves(|leaf_candidate| { + let CandidateState::Complete { success_block, continue_matching_block } = + leaf_candidate.state + else { + bug!() + }; + leaf_candidate.state = CandidateState::Incomplete; // At this point the leaf's own match pairs have all been lowered // and removed, so `extend` and assignment are equivalent, // but extending can also recycle any existing vector capacity. @@ -2110,13 +2180,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // The candidate was processed, and matching of the processed match pairs branched to // its `pre_binding_block`. We therefore start from that to match the new match pairs. - let or_start = leaf_candidate.pre_binding_block.unwrap(); + let or_start = success_block; // In a case like `(P | Q, R | S)`, if `P` succeeds and `R | S` fails, we know `(Q, R | // S)` will fail too. If there is no guard, we skip testing of `Q` by branching directly // to `last_otherwise`. If there is a guard, `leaf_candidate.otherwise_block` can be // reached by guard failure as well, so we can't skip `Q`. let or_otherwise = if leaf_candidate.has_guard { - leaf_candidate.otherwise_block.unwrap() + continue_matching_block } else { last_otherwise.unwrap() }; From 48c7313770b6275fd54a281ddf410fcb2020fc8b Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 28 Jul 2024 16:30:57 +0200 Subject: [PATCH 6/8] Include match pairs in the candidate state tracking --- .../rustc_mir_build/src/build/matches/mod.rs | 321 +++++++++++------- .../rustc_mir_build/src/build/matches/test.rs | 30 +- .../rustc_mir_build/src/build/matches/util.rs | 4 +- 3 files changed, 225 insertions(+), 130 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index ca2551c98a781..d23dc0c0f46ce 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1039,7 +1039,30 @@ impl<'tcx, 'pat> FlatPat<'pat, 'tcx> { #[derive(Debug)] enum CandidateState<'pat, 'tcx> { /// Still some match pairs to process. - Incomplete, + Incomplete { + /// For the candidate to match, all of these must be satisfied. + /// + /// --- + /// Initially contains a list of match pairs created by [`FlatPat`], but is + /// subsequently mutated (in a queue-like way) while lowering the match tree. + /// When this list becomes empty, the candidate is fully matched and becomes + /// a leaf (see [`Builder::select_matched_candidate`]). + /// + /// Key mutations include: + /// + /// - When a match pair is fully satisfied by a test, it is removed from the + /// list, and its subpairs are added instead (see [`Builder::sort_candidate`]). + /// - During or-pattern expansion, any leading or-pattern is removed, and is + /// converted into subcandidates (see [`Builder::expand_and_match_or_candidates`]). + /// - After a candidate's subcandidates have been lowered, a copy of any remaining + /// or-patterns is added to each leaf subcandidate + /// (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>, + }, /// This candidate has been fully processed. Complete { /// When the scrutinee matches, we branch to this block. @@ -1053,6 +1076,16 @@ enum CandidateState<'pat, 'tcx> { /// continue matching subsequent candidates. continue_matching_block: BasicBlock, }, + /// This candidate was expanded into or-patterns and then merged back into a single `candidate`. + /// The given match pairs must be processed starting from `success_block`, branching to + /// `failure_block` on failure. + /// + /// Invariant: `match_pairs` must not be empty. + PartiallyMatched { + success_block: BasicBlock, + failure_block: BasicBlock, + match_pairs: Vec>, + }, /// This candidate had an or-pattern and got expanded into subcandidates. Expanded { /// The span of the or-pattern they came from. @@ -1068,6 +1101,9 @@ enum CandidateState<'pat, 'tcx> { /// - See [`Builder::remove_never_subcandidates`]. /// Invariant: this must not be empty. subcandidates: Vec>, + /// Any match pairs that may remain. They will be processed in + /// `test_remaining_match_pairs_after_or`. + remaining_match_pairs: Vec>, }, } @@ -1083,29 +1119,6 @@ enum CandidateState<'pat, 'tcx> { /// the arm pattern to successfully match. #[derive(Debug)] struct Candidate<'pat, 'tcx> { - /// For the candidate to match, all of these must be satisfied. - /// - /// --- - /// Initially contains a list of match pairs created by [`FlatPat`], but is - /// subsequently mutated (in a queue-like way) while lowering the match tree. - /// When this list becomes empty, the candidate is fully matched and becomes - /// a leaf (see [`Builder::select_matched_candidate`]). - /// - /// Key mutations include: - /// - /// - When a match pair is fully satisfied by a test, it is removed from the - /// list, and its subpairs are added instead (see [`Builder::sort_candidate`]). - /// - During or-pattern expansion, any leading or-pattern is removed, and is - /// converted into subcandidates (see [`Builder::expand_and_match_or_candidates`]). - /// - After a candidate's subcandidates have been lowered, a copy of any remaining - /// or-patterns is added to each leaf subcandidate - /// (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>, - /// A `Candidate` goes through several states during the algorithm. See `CandidateState` for /// details. state: CandidateState<'pat, 'tcx>, @@ -1146,8 +1159,7 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> { /// 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, - state: CandidateState::Incomplete, + state: CandidateState::Incomplete { match_pairs: flat_pat.match_pairs }, extra_data: flat_pat.extra_data, has_guard, false_edge_start_block: None, @@ -1158,9 +1170,17 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> { self.state.subcandidates_mut().into_iter().flatten() } + /// Returns the match pairs if the candidate is `Incomplete`. + fn match_pairs(&self) -> Option<&[MatchPairTree<'pat, 'tcx>]> { + self.state.match_pairs() + } + /// 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 { test_case: TestCase::Or { .. }, .. }, ..]) + matches!( + self.match_pairs(), + Some([MatchPairTree { test_case: TestCase::Or { .. }, .. }, ..]) + ) } /// Visit the leaf candidates (those with no subcandidates) contained in @@ -1190,17 +1210,30 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> { impl<'pat, 'tcx> CandidateState<'pat, 'tcx> { fn into_subcandidates(self) -> Option>> { match self { - CandidateState::Incomplete | CandidateState::Complete { .. } => None, + CandidateState::Incomplete { .. } + | CandidateState::Complete { .. } + | CandidateState::PartiallyMatched { .. } => None, CandidateState::Expanded { subcandidates, .. } => Some(subcandidates), } } fn subcandidates_mut(&mut self) -> Option<&mut [Candidate<'pat, 'tcx>]> { match self { - CandidateState::Incomplete | CandidateState::Complete { .. } => None, + CandidateState::Incomplete { .. } + | CandidateState::Complete { .. } + | CandidateState::PartiallyMatched { .. } => None, CandidateState::Expanded { subcandidates, .. } => Some(subcandidates.as_mut_slice()), } } + + fn match_pairs(&self) -> Option<&[MatchPairTree<'pat, 'tcx>]> { + match self { + CandidateState::Expanded { .. } + | CandidateState::Complete { .. } + | CandidateState::PartiallyMatched { .. } => None, + CandidateState::Incomplete { match_pairs, .. } => Some(match_pairs), + } + } } /// A depth-first traversal of the `Candidate` and all of its recursive @@ -1226,9 +1259,9 @@ fn traverse_candidate<'pat, 'tcx: 'pat, C, T, I>( I: Iterator, { match candidate.borrow().state { - CandidateState::Incomplete | CandidateState::Complete { .. } => { - visit_leaf(candidate, context) - } + CandidateState::Incomplete { .. } + | CandidateState::Complete { .. } + | CandidateState::PartiallyMatched { .. } => visit_leaf(candidate, context), CandidateState::Expanded { .. } => { for child in get_children(candidate, context) { traverse_candidate(child, context, visit_leaf, get_children, complete_children); @@ -1485,7 +1518,6 @@ impl<'tcx> MatchTreeSubBranch<'tcx> { candidate: Candidate<'_, 'tcx>, parent_data: &Vec>, ) -> Self { - debug_assert!(candidate.match_pairs.is_empty()); let CandidateState::Complete { success_block, continue_matching_block } = candidate.state else { bug!("a leaf candidate should be `Complete` at the end of the algorithm") @@ -1597,7 +1629,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let CandidateState::Complete { success_block, continue_matching_block } = &mut leaf_candidate.state else { - bug!() + bug!("{leaf_candidate:?}") }; let source_info = self.source_info(leaf_candidate.extra_data.span); // Falsely branch to `next_candidate_start_block` before reaching pre_binding. @@ -1726,6 +1758,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// /// Note how we test `x` twice. This is the tradeoff of backtracking automata: we prefer smaller /// code size so we accept non-optimal code paths. + /// + /// Invariant: all the `candidates` must have state `Incomplete` at the start of the function, + /// and `Complete` or `Expanded` at the end. #[instrument(skip(self), level = "debug")] fn match_candidates( &mut self, @@ -1734,9 +1769,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { start_block: BasicBlock, candidates: &mut [&mut Candidate<'_, 'tcx>], ) -> BasicBlock { - ensure_sufficient_stack(|| { + debug_assert!( + candidates + .iter() + .all(|candidate| matches!(candidate.state, CandidateState::Incomplete { .. })) + ); + let otherwise_block = ensure_sufficient_stack(|| { self.match_candidates_inner(span, scrutinee_span, start_block, candidates) - }) + }); + debug_assert!(candidates.iter().all(|candidate| matches!( + candidate.state, + CandidateState::Complete { .. } | CandidateState::Expanded { .. } + ))); + otherwise_block } /// Construct the decision tree for `candidates`. Don't call this, call `match_candidates` @@ -1760,7 +1805,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // If there are no candidates that still need testing, we're done. return start_block; } - [first, remaining @ ..] if first.match_pairs.is_empty() => { + [first, remaining @ ..] + if let CandidateState::Incomplete { match_pairs, .. } = &first.state + && match_pairs.is_empty() => + { // The first candidate has satisfied all its match pairs. // We record the blocks that will be needed by match arm lowering, // and then continue with the remaining candidates. @@ -1818,8 +1866,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate: &mut Candidate<'_, 'tcx>, start_block: BasicBlock, ) -> BasicBlock { - assert!(matches!(candidate.state, CandidateState::Incomplete)); - // Create the otherwise block for this candidate, which is the // pre-binding block for the next candidate. let otherwise_block = self.cfg.start_new_block(); @@ -1870,7 +1916,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .position(|candidate| { // If a candidate starts with an or-pattern and has more match pairs, // we can expand it, but we must stop expanding _after_ it. - candidate.match_pairs.len() > 1 && candidate.starts_with_or_pattern() + candidate.starts_with_or_pattern() && candidate.match_pairs().unwrap().len() > 1 }) .map(|pos| pos + 1) // Stop _after_ the found candidate .unwrap_or(candidates.len()); // Otherwise, include all candidates @@ -1882,9 +1928,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let mut expanded_candidates = Vec::new(); for candidate in candidates_to_expand.iter_mut() { if candidate.starts_with_or_pattern() { - let or_match_pair = candidate.match_pairs.remove(0); // Expand the or-pattern into subcandidates. - self.create_or_subcandidates(candidate, or_match_pair); + self.expand_first_or_pattern(candidate); // Collect the newly created subcandidates. for subcandidate in candidate.subcandidates_mut() { expanded_candidates.push(subcandidate); @@ -1912,7 +1957,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // (Only the last candidate can possibly have more match pairs.) debug_assert!({ let mut all_except_last = candidates_to_expand.iter().rev().skip(1); - all_except_last.all(|candidate| candidate.match_pairs.is_empty()) + all_except_last + .all(|candidate| matches!(candidate.state, CandidateState::Complete { .. })) }); for candidate in candidates_to_expand.iter_mut() { self.merge_trivial_subcandidates(candidate); @@ -1931,12 +1977,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// Given a match-pair that corresponds to an or-pattern, expand each subpattern into a new /// subcandidate. Any candidate that has been expanded this way should also be postprocessed /// at the end of [`Self::expand_and_match_or_candidates`]. - fn create_or_subcandidates<'pat>( - &mut self, - candidate: &mut Candidate<'pat, 'tcx>, - match_pair: MatchPairTree<'pat, 'tcx>, - ) { - assert!(matches!(candidate.state, CandidateState::Incomplete)); + fn expand_first_or_pattern<'pat>(&mut self, candidate: &mut Candidate<'pat, 'tcx>) { + let CandidateState::Incomplete { match_pairs, .. } = &mut candidate.state else { bug!() }; + let match_pair = match_pairs.remove(0); + let remaining_match_pairs = mem::take(match_pairs); let TestCase::Or { pats, .. } = match_pair.test_case else { bug!() }; debug!("expanding or-pattern: candidate={:#?}\npats={:#?}", candidate, pats); let or_span = match_pair.pattern.span; @@ -1946,7 +1990,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .map(|flat_pat| Candidate::from_flat_pat(flat_pat, candidate.has_guard)) .collect(); subcandidates[0].false_edge_start_block = candidate.false_edge_start_block; - candidate.state = CandidateState::Expanded { or_span, subcandidates }; + candidate.state = + CandidateState::Expanded { or_span, subcandidates, remaining_match_pairs }; } /// Try to merge all of the subcandidates of the given candidate into one. This avoids @@ -2038,7 +2083,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// Note that this takes place _after_ the subcandidates have participated /// in match tree lowering. fn merge_trivial_subcandidates(&mut self, candidate: &mut Candidate<'_, 'tcx>) { - let CandidateState::Expanded { subcandidates, or_span } = &mut candidate.state else { + let CandidateState::Expanded { subcandidates, or_span, remaining_match_pairs, .. } = + &mut candidate.state + else { return; }; if candidate.has_guard { @@ -2079,10 +2126,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } // This is a lie, some match pairs may remain. They'll be processed right away by // `test_remaining_match_pairs_after_or` - candidate.state = CandidateState::Complete { - success_block: shared_pre_binding_block, - continue_matching_block: last_otherwise.unwrap(), - }; + if remaining_match_pairs.is_empty() { + candidate.state = CandidateState::Complete { + success_block: shared_pre_binding_block, + continue_matching_block: last_otherwise.unwrap(), + }; + } else { + let remaining_match_pairs = mem::take(remaining_match_pairs); + candidate.state = CandidateState::PartiallyMatched { + success_block: shared_pre_binding_block, + failure_block: last_otherwise.unwrap(), + match_pairs: remaining_match_pairs, + }; + } } /// Never subcandidates may have a set of bindings inconsistent with their siblings, @@ -2096,7 +2152,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if candidate.extra_data.is_never { candidate.visit_leaves(|subcandidate| { match &subcandidate.state { - CandidateState::Incomplete | CandidateState::Expanded { .. } => bug!(), + CandidateState::Incomplete { .. } + | CandidateState::PartiallyMatched { .. } + | CandidateState::Expanded { .. } => { + bug!() + } CandidateState::Complete { success_block, continue_matching_block } => { last_otherwise = Some(*continue_matching_block); // That block is already unreachable but needs a terminator to make the MIR well-formed. @@ -2133,67 +2193,99 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { scrutinee_span: Span, candidate: &mut Candidate<'_, 'tcx>, ) { - if candidate.match_pairs.is_empty() { - return; - } + match &mut candidate.state { + // Nothing to do. + CandidateState::Complete { .. } => {} + // Candidate was expanded then merged. We continue matching the remaining match pairs + // from where we left off. + CandidateState::PartiallyMatched { success_block, failure_block, match_pairs } => { + let remaining_match_pairs = mem::take(match_pairs); + let success_block = *success_block; + let failure_block = *failure_block; + + // We're testing match pairs that remained after an `Or`, so the remaining + // pairs should all be `Or` too, due to the sorting invariant. + debug_assert!( + remaining_match_pairs + .iter() + .all(|match_pair| matches!(match_pair.test_case, TestCase::Or { .. })) + ); - let or_span = match candidate.state { - CandidateState::Expanded { or_span, .. } => or_span, - // This branch is possible if we just merged trivial patterns. - _ => candidate.extra_data.span, - }; - let source_info = self.source_info(or_span); + // This candidate was partially processed. We resume processing it with the + // remaining match pairs. + candidate.state = + CandidateState::Incomplete { match_pairs: remaining_match_pairs.clone() }; + let otherwise = + self.match_candidates(span, scrutinee_span, success_block, &mut [candidate]); - let mut last_otherwise = None; - candidate.visit_leaves(|leaf_candidate| { - let CandidateState::Complete { continue_matching_block, .. } = leaf_candidate.state - else { - bug!() - }; - last_otherwise = Some(continue_matching_block); - }); + let source_info = self.source_info(candidate.extra_data.span); + self.cfg.goto(otherwise, source_info, failure_block); + } + // The candidate was expanded. If some match pairs remain, we process them inside each + // leaf candidate. + CandidateState::Expanded { or_span, remaining_match_pairs, .. } => { + if remaining_match_pairs.is_empty() { + // Nothing to do. + return; + } + let remaining_match_pairs = mem::take(remaining_match_pairs); + let source_info = self.source_info(*or_span); - let remaining_match_pairs = mem::take(&mut candidate.match_pairs); - // We're testing match pairs that remained after an `Or`, so the remaining - // pairs should all be `Or` too, due to the sorting invariant. - debug_assert!( - remaining_match_pairs - .iter() - .all(|match_pair| matches!(match_pair.test_case, TestCase::Or { .. })) - ); + let mut last_otherwise = None; + candidate.visit_leaves(|leaf_candidate| { + let CandidateState::Complete { continue_matching_block, .. } = + leaf_candidate.state + else { + bug!() + }; + last_otherwise = Some(continue_matching_block); + }); - // Visit each leaf candidate within this subtree, add a copy of the remaining - // match pairs to it, and then recursively lower the rest of the match tree - // from that point. - candidate.visit_leaves(|leaf_candidate| { - let CandidateState::Complete { success_block, continue_matching_block } = - leaf_candidate.state - else { - bug!() - }; - leaf_candidate.state = CandidateState::Incomplete; - // At this point the leaf's own match pairs have all been lowered - // and removed, so `extend` and assignment are equivalent, - // but extending can also recycle any existing vector capacity. - assert!(leaf_candidate.match_pairs.is_empty()); - leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned()); - - // The candidate was processed, and matching of the processed match pairs branched to - // its `pre_binding_block`. We therefore start from that to match the new match pairs. - let or_start = success_block; - // In a case like `(P | Q, R | S)`, if `P` succeeds and `R | S` fails, we know `(Q, R | - // S)` will fail too. If there is no guard, we skip testing of `Q` by branching directly - // to `last_otherwise`. If there is a guard, `leaf_candidate.otherwise_block` can be - // reached by guard failure as well, so we can't skip `Q`. - let or_otherwise = if leaf_candidate.has_guard { - continue_matching_block - } else { - last_otherwise.unwrap() - }; - let otherwise = - self.match_candidates(span, scrutinee_span, or_start, &mut [leaf_candidate]); - self.cfg.goto(otherwise, source_info, or_otherwise); - }); + // We're testing match pairs that remained after an `Or`, so the remaining + // pairs should all be `Or` too, due to the sorting invariant. + debug_assert!( + remaining_match_pairs + .iter() + .all(|match_pair| matches!(match_pair.test_case, TestCase::Or { .. })) + ); + + // Visit each leaf candidate within this subtree, add a copy of the remaining + // match pairs to it, and then recursively lower the rest of the match tree + // from that point. + candidate.visit_leaves(|leaf_candidate| { + // This candidate was fully processed. We resume processing it with the remaining match + // pairs. + let CandidateState::Complete { success_block, continue_matching_block } = + leaf_candidate.state + else { + bug!() + }; + leaf_candidate.state = + CandidateState::Incomplete { match_pairs: remaining_match_pairs.clone() }; + + // The candidate was processed, and matching of the processed match pairs branched to + // its `pre_binding_block`. We therefore start from that to match the new match pairs. + let start_block = success_block; + // In a case like `(P | Q, R | S)`, if `P` succeeds and `R | S` fails, we know `(Q, R | + // S)` will fail too. If there is no guard, we skip testing of `Q` by branching directly + // to `last_otherwise`. If there is a guard, `leaf_candidate.otherwise_block` can be + // reached by guard failure as well, so we can't skip `Q`. + let otherwise_block = if leaf_candidate.has_guard { + continue_matching_block + } else { + last_otherwise.unwrap() + }; + let otherwise = self.match_candidates( + span, + scrutinee_span, + start_block, + &mut [leaf_candidate], + ); + self.cfg.goto(otherwise, source_info, otherwise_block); + }); + } + CandidateState::Incomplete { .. } => bug!(), + } } /// Pick a test to run. Which test doesn't matter as long as it is guaranteed to fully match at @@ -2214,8 +2306,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// [`SwitchInt`]: TestKind::SwitchInt /// [`Range`]: TestKind::Range fn pick_test(&mut self, candidates: &[&mut Candidate<'_, 'tcx>]) -> (Place<'tcx>, Test<'tcx>) { + let CandidateState::Incomplete { match_pairs, .. } = &candidates[0].state else { bug!() }; // Extract the match-pair from the highest priority candidate - let match_pair = &candidates[0].match_pairs[0]; + let match_pair = &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(); diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 7af1ede24a4f4..1f4e63aca9d98 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -20,7 +20,9 @@ use rustc_span::symbol::{sym, Symbol}; use rustc_span::{Span, DUMMY_SP}; use tracing::{debug, instrument}; -use crate::build::matches::{Candidate, MatchPairTree, Test, TestBranch, TestCase, TestKind}; +use crate::build::matches::{ + Candidate, CandidateState, MatchPairTree, Test, TestBranch, TestCase, TestKind, +}; use crate::build::Builder; impl<'a, 'tcx> Builder<'a, 'tcx> { @@ -535,17 +537,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate: &mut Candidate<'_, 'tcx>, sorted_candidates: &FxIndexMap, Vec<&mut Candidate<'_, 'tcx>>>, ) -> Option> { + let CandidateState::Incomplete { match_pairs, .. } = &mut candidate.state else { bug!() }; // Find the match_pair for this place (if any). At present, // afaik, there can be at most one. (In the future, if we // adopted a more general `@` operator, there might be more // 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) = + match_pairs.iter().enumerate().find(|&(_, mp)| mp.place == Some(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_ @@ -581,12 +581,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { matches!(range.contains(value, self.tcx, self.param_env), None | Some(true)) }) }; - let is_conflicting_candidate = |candidate: &&mut Candidate<'_, 'tcx>| { - candidate - .match_pairs - .iter() - .any(|mp| mp.place == Some(test_place) && is_covering_range(&mp.test_case)) - }; + let is_conflicting_candidate = + |candidate: &&mut Candidate<'_, 'tcx>| { + candidate.match_pairs().unwrap().iter().any(|mp| { + mp.place == Some(test_place) && is_covering_range(&mp.test_case) + }) + }; if sorted_candidates .get(&TestBranch::Failure) .is_some_and(|candidates| candidates.iter().any(is_conflicting_candidate)) @@ -756,10 +756,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 match_pair = match_pairs.remove(match_pair_index); + 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 { .. })); + match_pairs.sort_by_key(|pair| matches!(pair.test_case, TestCase::Or { .. })); } ret diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 8491b5fe380c7..24b157faf8741 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -140,7 +140,9 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { for binding in &candidate.extra_data.bindings { self.visit_binding(binding); } - for match_pair in &candidate.match_pairs { + // This assumes the candidate state is `Incomplete`, which is the case since we collect fake + // borrows before starting the processing of candidates. + for match_pair in candidate.match_pairs().unwrap() { self.visit_match_pair(match_pair); } } From 38c33f9d2862d0efe92aec5ee7ec3eb5efc6d6a1 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 29 Jul 2024 17:30:39 +0200 Subject: [PATCH 7/8] Decide whether to merge subcandidates ahead of time --- .../rustc_mir_build/src/build/matches/mod.rs | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index d23dc0c0f46ce..59d1ea01cf6b7 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1101,6 +1101,8 @@ enum CandidateState<'pat, 'tcx> { /// - See [`Builder::remove_never_subcandidates`]. /// Invariant: this must not be empty. subcandidates: Vec>, + /// Whether the subcandidates should be merged together after they've been processed. + simplify: bool, /// Any match pairs that may remain. They will be processed in /// `test_remaining_match_pairs_after_or`. remaining_match_pairs: Vec>, @@ -1981,9 +1983,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let CandidateState::Incomplete { match_pairs, .. } = &mut candidate.state else { bug!() }; let match_pair = match_pairs.remove(0); let remaining_match_pairs = mem::take(match_pairs); - let TestCase::Or { pats, .. } = match_pair.test_case else { bug!() }; + let TestCase::Or { pats, contains_bindings } = match_pair.test_case else { bug!() }; debug!("expanding or-pattern: candidate={:#?}\npats={:#?}", candidate, pats); let or_span = match_pair.pattern.span; + let simplify = !candidate.has_guard && !contains_bindings; let mut subcandidates: Vec<_> = pats .into_vec() .into_iter() @@ -1991,7 +1994,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .collect(); subcandidates[0].false_edge_start_block = candidate.false_edge_start_block; candidate.state = - CandidateState::Expanded { or_span, subcandidates, remaining_match_pairs }; + CandidateState::Expanded { or_span, subcandidates, simplify, remaining_match_pairs }; } /// Try to merge all of the subcandidates of the given candidate into one. This avoids @@ -2083,24 +2086,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// Note that this takes place _after_ the subcandidates have participated /// in match tree lowering. fn merge_trivial_subcandidates(&mut self, candidate: &mut Candidate<'_, 'tcx>) { - let CandidateState::Expanded { subcandidates, or_span, remaining_match_pairs, .. } = - &mut candidate.state + let CandidateState::Expanded { + subcandidates, + or_span, + simplify: true, + remaining_match_pairs, + .. + } = &mut candidate.state else { return; }; - if candidate.has_guard { - // FIXME(or_patterns; matthewjasper) Don't give up if we have a guard. - return; - } - - // FIXME(or_patterns; matthewjasper) Try to be more aggressive here. - let can_merge = subcandidates.iter().all(|subcandidate| { - !matches!(subcandidate.state, CandidateState::Expanded { .. }) - && subcandidate.extra_data.is_empty() - }); - if !can_merge { - return; - } let mut last_otherwise = None; let shared_pre_binding_block = self.cfg.start_new_block(); From 65b2ae412eb0defd0b7f116863a809f91486c04d Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 29 Jul 2024 18:55:05 +0200 Subject: [PATCH 8/8] Merge the two steps of or-pattern post-processing into one --- .../rustc_mir_build/src/build/matches/mod.rs | 269 ++++++++---------- 1 file changed, 114 insertions(+), 155 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 59d1ea01cf6b7..c16e24d3d7ac0 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1076,16 +1076,6 @@ enum CandidateState<'pat, 'tcx> { /// continue matching subsequent candidates. continue_matching_block: BasicBlock, }, - /// This candidate was expanded into or-patterns and then merged back into a single `candidate`. - /// The given match pairs must be processed starting from `success_block`, branching to - /// `failure_block` on failure. - /// - /// Invariant: `match_pairs` must not be empty. - PartiallyMatched { - success_block: BasicBlock, - failure_block: BasicBlock, - match_pairs: Vec>, - }, /// This candidate had an or-pattern and got expanded into subcandidates. Expanded { /// The span of the or-pattern they came from. @@ -1212,27 +1202,21 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> { impl<'pat, 'tcx> CandidateState<'pat, 'tcx> { fn into_subcandidates(self) -> Option>> { match self { - CandidateState::Incomplete { .. } - | CandidateState::Complete { .. } - | CandidateState::PartiallyMatched { .. } => None, + CandidateState::Incomplete { .. } | CandidateState::Complete { .. } => None, CandidateState::Expanded { subcandidates, .. } => Some(subcandidates), } } fn subcandidates_mut(&mut self) -> Option<&mut [Candidate<'pat, 'tcx>]> { match self { - CandidateState::Incomplete { .. } - | CandidateState::Complete { .. } - | CandidateState::PartiallyMatched { .. } => None, + CandidateState::Incomplete { .. } | CandidateState::Complete { .. } => None, CandidateState::Expanded { subcandidates, .. } => Some(subcandidates.as_mut_slice()), } } fn match_pairs(&self) -> Option<&[MatchPairTree<'pat, 'tcx>]> { match self { - CandidateState::Expanded { .. } - | CandidateState::Complete { .. } - | CandidateState::PartiallyMatched { .. } => None, + CandidateState::Expanded { .. } | CandidateState::Complete { .. } => None, CandidateState::Incomplete { match_pairs, .. } => Some(match_pairs), } } @@ -1261,9 +1245,9 @@ fn traverse_candidate<'pat, 'tcx: 'pat, C, T, I>( I: Iterator, { match candidate.borrow().state { - CandidateState::Incomplete { .. } - | CandidateState::Complete { .. } - | CandidateState::PartiallyMatched { .. } => visit_leaf(candidate, context), + CandidateState::Incomplete { .. } | CandidateState::Complete { .. } => { + visit_leaf(candidate, context) + } CandidateState::Expanded { .. } => { for child in get_children(candidate, context) { traverse_candidate(child, context, visit_leaf, get_children, complete_children); @@ -1963,14 +1947,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .all(|candidate| matches!(candidate.state, CandidateState::Complete { .. })) }); for candidate in candidates_to_expand.iter_mut() { - self.merge_trivial_subcandidates(candidate); self.remove_never_subcandidates(candidate); - } - // It's important to perform the above simplifications _before_ dealing - // with remaining match pairs, to avoid exponential blowup if possible - // (for trivial or-patterns), and avoid useless work (for never patterns). - if let Some(last_candidate) = candidates_to_expand.last_mut() { - self.test_remaining_match_pairs_after_or(span, scrutinee_span, last_candidate); + self.post_process_expanded_candidates(span, scrutinee_span, candidate); } remainder_start.and(remaining_candidates) @@ -1997,9 +1975,47 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { CandidateState::Expanded { or_span, subcandidates, simplify, remaining_match_pairs }; } - /// Try to merge all of the subcandidates of the given candidate into one. This avoids - /// exponentially large CFGs in cases like `(1 | 2, 3 | 4, ...)`. The candidate should have been - /// expanded with `create_or_subcandidates`. + /// Never subcandidates may have a set of bindings inconsistent with their siblings, + /// which would break later code. So we filter them out. Note that we can't filter out + /// top-level candidates this way. + fn remove_never_subcandidates(&mut self, candidate: &mut Candidate<'_, 'tcx>) { + let CandidateState::Expanded { subcandidates, .. } = &mut candidate.state else { return }; + + let mut last_otherwise = None; + subcandidates.retain_mut(|candidate| { + if candidate.extra_data.is_never { + candidate.visit_leaves(|subcandidate| { + match &subcandidate.state { + CandidateState::Incomplete { .. } | CandidateState::Expanded { .. } => { + bug!() + } + CandidateState::Complete { success_block, continue_matching_block } => { + last_otherwise = Some(*continue_matching_block); + // That block is already unreachable but needs a terminator to make the MIR well-formed. + let source_info = self.source_info(subcandidate.extra_data.span); + self.cfg.terminate( + *success_block, + source_info, + TerminatorKind::Unreachable, + ); + } + } + }); + false + } else { + true + } + }); + if subcandidates.is_empty() { + // If `candidate` has become a leaf candidate, update its state. + candidate.state = CandidateState::Complete { + success_block: self.cfg.start_new_block(), + continue_matching_block: last_otherwise.unwrap(), + }; + } + } + + /// Continue processing a candidate that was expanded into subcandidates. /// /// Given a pattern `(P | Q, R | S)` we would like to generate a CFG like so: /// @@ -2084,105 +2100,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// ``` /// /// Note that this takes place _after_ the subcandidates have participated - /// in match tree lowering. - fn merge_trivial_subcandidates(&mut self, candidate: &mut Candidate<'_, 'tcx>) { - let CandidateState::Expanded { - subcandidates, - or_span, - simplify: true, - remaining_match_pairs, - .. - } = &mut candidate.state - else { - return; - }; - - let mut last_otherwise = None; - let shared_pre_binding_block = self.cfg.start_new_block(); - let source_info = self.source_info(*or_span); - - if candidate.false_edge_start_block.is_none() { - candidate.false_edge_start_block = subcandidates[0].false_edge_start_block; - } - - // Remove the (known-trivial) subcandidates from the candidate tree, - // so that they aren't visible after match tree lowering, and wire them - // all to join up at a single shared pre-binding block. - // (Note that the subcandidates have already had their part of the match - // tree lowered by this point, which is why we can add a goto to them.) - for subcandidate in mem::take(subcandidates) { - let CandidateState::Complete { success_block, continue_matching_block } = - subcandidate.state - else { - bug!() - }; - self.cfg.goto(success_block, source_info, shared_pre_binding_block); - last_otherwise = Some(continue_matching_block); - } - // This is a lie, some match pairs may remain. They'll be processed right away by - // `test_remaining_match_pairs_after_or` - if remaining_match_pairs.is_empty() { - candidate.state = CandidateState::Complete { - success_block: shared_pre_binding_block, - continue_matching_block: last_otherwise.unwrap(), - }; - } else { - let remaining_match_pairs = mem::take(remaining_match_pairs); - candidate.state = CandidateState::PartiallyMatched { - success_block: shared_pre_binding_block, - failure_block: last_otherwise.unwrap(), - match_pairs: remaining_match_pairs, - }; - } - } - - /// Never subcandidates may have a set of bindings inconsistent with their siblings, - /// which would break later code. So we filter them out. Note that we can't filter out - /// top-level candidates this way. - fn remove_never_subcandidates(&mut self, candidate: &mut Candidate<'_, 'tcx>) { - let CandidateState::Expanded { subcandidates, .. } = &mut candidate.state else { return }; - - let mut last_otherwise = None; - subcandidates.retain_mut(|candidate| { - if candidate.extra_data.is_never { - candidate.visit_leaves(|subcandidate| { - match &subcandidate.state { - CandidateState::Incomplete { .. } - | CandidateState::PartiallyMatched { .. } - | CandidateState::Expanded { .. } => { - bug!() - } - CandidateState::Complete { success_block, continue_matching_block } => { - last_otherwise = Some(*continue_matching_block); - // That block is already unreachable but needs a terminator to make the MIR well-formed. - let source_info = self.source_info(subcandidate.extra_data.span); - self.cfg.terminate( - *success_block, - source_info, - TerminatorKind::Unreachable, - ); - } - } - }); - false - } else { - true - } - }); - if subcandidates.is_empty() { - // If `candidate` has become a leaf candidate, update its state. - candidate.state = CandidateState::Complete { - success_block: self.cfg.start_new_block(), - continue_matching_block: last_otherwise.unwrap(), - }; - } - } - - /// If more match pairs remain, test them after each subcandidate. - /// We could have added them to the or-candidates during or-pattern expansion, but that - /// would make it impossible to detect simplifiable or-patterns. That would guarantee - /// exponentially large CFGs for cases like `(1 | 2, 3 | 4, ...)`. - fn test_remaining_match_pairs_after_or( + /// in match tree lowering, hence why the `Incomplete` state would be a bug. + fn post_process_expanded_candidates( &mut self, span: Span, scrutinee_span: Span, @@ -2191,34 +2110,73 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match &mut candidate.state { // Nothing to do. CandidateState::Complete { .. } => {} - // Candidate was expanded then merged. We continue matching the remaining match pairs - // from where we left off. - CandidateState::PartiallyMatched { success_block, failure_block, match_pairs } => { - let remaining_match_pairs = mem::take(match_pairs); - let success_block = *success_block; - let failure_block = *failure_block; + // Merge all of the subcandidates of this candidate into one. This avoids exponentially + // large CFGs in cases like `(1 | 2, 3 | 4, ...)`. If some match pairs remain, we + // process them afterwards. + CandidateState::Expanded { + subcandidates, + or_span, + remaining_match_pairs, + simplify: true, + .. + } => { + let shared_pre_binding_block = self.cfg.start_new_block(); + let source_info = self.source_info(*or_span); + if candidate.false_edge_start_block.is_none() { + candidate.false_edge_start_block = subcandidates[0].false_edge_start_block; + } - // We're testing match pairs that remained after an `Or`, so the remaining - // pairs should all be `Or` too, due to the sorting invariant. - debug_assert!( - remaining_match_pairs - .iter() - .all(|match_pair| matches!(match_pair.test_case, TestCase::Or { .. })) - ); + // Remove the (known-trivial) subcandidates from the candidate tree, + // so that they aren't visible after match tree lowering, and wire them + // all to join up at a single shared pre-binding block. + // (Note that the subcandidates have already had their part of the match + // tree lowered by this point, which is why we can add a goto to them.) + let mut last_otherwise = None; + for subcandidate in mem::take(subcandidates) { + let CandidateState::Complete { success_block, continue_matching_block } = + subcandidate.state + else { + bug!() + }; + self.cfg.goto(success_block, source_info, shared_pre_binding_block); + last_otherwise = Some(continue_matching_block); + } + + let success_block = shared_pre_binding_block; + let failure_block = last_otherwise.unwrap(); + if remaining_match_pairs.is_empty() { + candidate.state = CandidateState::Complete { + success_block, + continue_matching_block: failure_block, + }; + } else { + // We're testing match pairs that remained after an `Or`, so the remaining + // pairs should all be `Or` too, due to the sorting invariant. + debug_assert!( + remaining_match_pairs + .iter() + .all(|match_pair| matches!(match_pair.test_case, TestCase::Or { .. })) + ); - // This candidate was partially processed. We resume processing it with the - // remaining match pairs. - candidate.state = - CandidateState::Incomplete { match_pairs: remaining_match_pairs.clone() }; - let otherwise = - self.match_candidates(span, scrutinee_span, success_block, &mut [candidate]); + // We resume processing with the remaining match pairs. + candidate.state = + CandidateState::Incomplete { match_pairs: remaining_match_pairs.clone() }; + let otherwise = self.match_candidates( + span, + scrutinee_span, + success_block, + &mut [candidate], + ); - let source_info = self.source_info(candidate.extra_data.span); - self.cfg.goto(otherwise, source_info, failure_block); + let source_info = self.source_info(candidate.extra_data.span); + self.cfg.goto(otherwise, source_info, failure_block); + } } - // The candidate was expanded. If some match pairs remain, we process them inside each - // leaf candidate. - CandidateState::Expanded { or_span, remaining_match_pairs, .. } => { + // The candidate was expanded, and we can't simplify it. If some match pairs remain, we + // process them inside each leaf candidate. + CandidateState::Expanded { + or_span, remaining_match_pairs, simplify: false, .. + } => { if remaining_match_pairs.is_empty() { // Nothing to do. return; @@ -2279,6 +2237,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.cfg.goto(otherwise, source_info, otherwise_block); }); } + // This should only be called on processed candidates. CandidateState::Incomplete { .. } => bug!(), } }