diff --git a/compiler/rustc_mir_build/src/builder/matches/buckets.rs b/compiler/rustc_mir_build/src/builder/matches/buckets.rs index 0643704ed35a..f53e73d406c3 100644 --- a/compiler/rustc_mir_build/src/builder/matches/buckets.rs +++ b/compiler/rustc_mir_build/src/builder/matches/buckets.rs @@ -9,6 +9,14 @@ use crate::builder::Builder; use crate::builder::matches::test::is_switch_ty; use crate::builder::matches::{Candidate, Test, TestBranch, TestCase, TestKind}; +/// Output of [`Builder::partition_candidates_into_buckets`]. +pub(crate) struct PartitionedCandidates<'tcx, 'b, 'c> { + /// For each possible outcome of the test, the candidates that are matched in that outcome. + pub(crate) target_candidates: FxIndexMap, Vec<&'b mut Candidate<'tcx>>>, + /// The remaining candidates that weren't associated with any test outcome. + pub(crate) remaining_candidates: &'b mut [&'c mut Candidate<'tcx>], +} + impl<'a, 'tcx> Builder<'a, 'tcx> { /// Given a test, we partition the input candidates into several buckets. /// If a candidate matches in exactly one of the branches of `test` @@ -20,10 +28,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// that are entailed by the outcome of the test, and add any sub-pairs of the /// removed pairs. /// - /// This returns a pair of - /// - the candidates that weren't sorted; - /// - for each possible outcome of the test, the candidates that match in that outcome. - /// /// For example: /// ``` /// # let (x, y, z) = (true, true, true); @@ -41,36 +45,33 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// - If the outcome is that `x` is false, candidates {1, 2} are possible /// /// Following our algorithm: - /// - Candidate 0 is sorted into outcome `x == true` - /// - Candidate 1 is sorted into outcome `x == false` - /// - Candidate 2 remains unsorted, because testing `x` has no effect on it - /// - Candidate 3 remains unsorted, because a previous candidate (2) was unsorted + /// - Candidate 0 is bucketed into outcome `x == true` + /// - Candidate 1 is bucketed into outcome `x == false` + /// - Candidate 2 remains unbucketed, because testing `x` has no effect on it + /// - Candidate 3 remains unbucketed, because a previous candidate (2) was unbucketed /// - This helps preserve the illusion that candidates are tested "in order" /// - /// The sorted candidates are mutated to remove entailed match pairs: + /// The bucketed candidates are mutated to remove entailed match pairs: /// - candidate 0 becomes `[z @ true]` since we know that `x` was `true`; /// - candidate 1 becomes `[y @ false]` since we know that `x` was `false`. - pub(super) fn sort_candidates<'b, 'c>( + pub(super) fn partition_candidates_into_buckets<'b, 'c>( &mut self, match_place: Place<'tcx>, test: &Test<'tcx>, mut candidates: &'b mut [&'c mut Candidate<'tcx>], - ) -> ( - &'b mut [&'c mut Candidate<'tcx>], - FxIndexMap, Vec<&'b mut Candidate<'tcx>>>, - ) { - // For each of the possible outcomes, collect vector of candidates that apply if the test + ) -> PartitionedCandidates<'tcx, 'b, 'c> { + // For each of the possible outcomes, collect a vector of candidates that apply if the test // has that particular outcome. let mut target_candidates: FxIndexMap<_, Vec<&mut Candidate<'_>>> = Default::default(); let total_candidate_count = candidates.len(); - // Sort the candidates into the appropriate vector in `target_candidates`. Note that at some - // point we may encounter a candidate where the test is not relevant; at that point, we stop - // sorting. + // Partition the candidates into the appropriate vector in `target_candidates`. + // Note that at some point we may encounter a candidate where the test is not relevant; + // at that point, we stop partitioning. while let Some(candidate) = candidates.first_mut() { let Some(branch) = - self.sort_candidate(match_place, test, candidate, &target_candidates) + self.choose_bucket_for_candidate(match_place, test, candidate, &target_candidates) else { break; }; @@ -87,7 +88,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { debug!("tested_candidates: {}", total_candidate_count - candidates.len()); debug!("untested_candidates: {}", candidates.len()); - (candidates, target_candidates) + PartitionedCandidates { target_candidates, remaining_candidates: candidates } } /// Given that we are performing `test` against `test_place`, this job @@ -115,12 +116,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// that it *doesn't* apply. For now, we return false, indicate that the /// test does not apply to this candidate, but it might be we can get /// tighter match code if we do something a bit different. - fn sort_candidate( + fn choose_bucket_for_candidate( &mut self, test_place: Place<'tcx>, test: &Test<'tcx>, candidate: &mut Candidate<'tcx>, - sorted_candidates: &FxIndexMap, Vec<&mut Candidate<'tcx>>>, + // Other candidates that have already been partitioned into a bucket for this test, if any + prior_candidates: &FxIndexMap, Vec<&mut Candidate<'tcx>>>, ) -> Option> { // Find the match_pair for this place (if any). At present, // afaik, there can be at most one. (In the future, if we @@ -155,13 +157,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // // FIXME(#29623) we could use PatKind::Range to rule // things out here, in some cases. + // + // FIXME(Zalathar): Is the `is_switch_ty` test unnecessary? (TestKind::SwitchInt, &TestCase::Constant { value }) if is_switch_ty(match_pair.pattern_ty) => { - // An important invariant of candidate sorting is that a candidate + // An important invariant of candidate bucketing is that a candidate // must not match in multiple branches. For `SwitchInt` tests, adding // a new value might invalidate that property for range patterns that - // have already been sorted into the failure arm, so we must take care + // have already been partitioned into the failure arm, so we must take care // not to add such values here. let is_covering_range = |test_case: &TestCase<'tcx>| { test_case.as_range().is_some_and(|range| { @@ -174,7 +178,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .iter() .any(|mp| mp.place == Some(test_place) && is_covering_range(&mp.test_case)) }; - if sorted_candidates + if prior_candidates .get(&TestBranch::Failure) .is_some_and(|candidates| candidates.iter().any(is_conflicting_candidate)) { @@ -191,7 +195,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // the values being tested. (This restricts what values can be // added to the test by subsequent candidates.) fully_matched = false; - let not_contained = sorted_candidates + let not_contained = prior_candidates .keys() .filter_map(|br| br.as_constant()) .all(|val| matches!(range.contains(val, self.tcx), Some(false))); diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index aa223435720c..5074ce336798 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -27,6 +27,7 @@ use tracing::{debug, instrument}; use crate::builder::ForGuard::{self, OutsideGuard, RefWithinGuard}; use crate::builder::expr::as_place::PlaceBuilder; +use crate::builder::matches::buckets::PartitionedCandidates; use crate::builder::matches::user_ty::ProjectedUserTypesNode; use crate::builder::scope::DropKind; use crate::builder::{ @@ -1069,7 +1070,7 @@ struct Candidate<'tcx> { /// 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`]). + /// list, and its subpairs are added instead (see [`Builder::choose_bucket_for_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 @@ -1254,7 +1255,7 @@ struct Ascription<'tcx> { /// /// Created by [`MatchPairTree::for_pattern`], and then inspected primarily by: /// - [`Builder::pick_test_for_match_pair`] (to choose a test) -/// - [`Builder::sort_candidate`] (to see how the test interacts with a match pair) +/// - [`Builder::choose_bucket_for_candidate`] (to see how the test interacts with a match pair) /// /// Note that or-patterns are not tested directly like the other variants. /// Instead they participate in or-pattern expansion, where they are transformed into @@ -2284,8 +2285,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // For each of the N possible test outcomes, build the vector of candidates that applies if // the test has that particular outcome. This also mutates the candidates to remove match // pairs that are fully satisfied by the relevant outcome. - let (remaining_candidates, target_candidates) = - self.sort_candidates(match_place, &test, candidates); + let PartitionedCandidates { target_candidates, remaining_candidates } = + self.partition_candidates_into_buckets(match_place, &test, candidates); // The block that we should branch to if none of the `target_candidates` match. let remainder_start = self.cfg.start_new_block();