1
Fork 0

Rollup merge of #121175 - Nadrieril:simplify-or-selection, r=matthewjasper

match lowering: test one or pattern at a time

This is a bit more opinionated than the previous PRs. On the face of it this is less efficient and more complex than before, but I personally found the loop that digs into `leaf_candidates` on each iteration very confusing. Instead this does "generate code for this or-pattern" then "generate further code for each branch if needed" in two steps.

Incidentally this way we don't _require_ or patterns to be sorted at the end. It's still an important optimization but I find it clearer to not rely on it for correctness.

r? `@matthewjasper`
This commit is contained in:
León Orell Valerian Liehr 2024-02-21 16:32:57 +01:00 committed by GitHub
commit 4daa43aaff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -1052,7 +1052,7 @@ struct Ascription<'tcx> {
variance: ty::Variance, variance: ty::Variance,
} }
#[derive(Debug)] #[derive(Debug, Clone)]
pub(crate) struct MatchPair<'pat, 'tcx> { pub(crate) struct MatchPair<'pat, 'tcx> {
// This place... // This place...
place: PlaceBuilder<'tcx>, place: PlaceBuilder<'tcx>,
@ -1408,51 +1408,66 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
span: Span, span: Span,
scrutinee_span: Span, scrutinee_span: Span,
candidates: &mut [&mut Candidate<'_, 'tcx>], candidates: &mut [&mut Candidate<'_, 'tcx>],
block: BasicBlock, start_block: BasicBlock,
otherwise_block: BasicBlock, otherwise_block: BasicBlock,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>, fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) { ) {
let (first_candidate, remaining_candidates) = candidates.split_first_mut().unwrap(); let (first_candidate, remaining_candidates) = candidates.split_first_mut().unwrap();
assert!(first_candidate.subcandidates.is_empty());
// All of the or-patterns have been sorted to the end, so if the first if !matches!(first_candidate.match_pairs[0].pattern.kind, PatKind::Or { .. }) {
// pattern is an or-pattern we only have or-patterns. self.test_candidates(
match first_candidate.match_pairs[0].pattern.kind { span,
PatKind::Or { .. } => (), scrutinee_span,
_ => { candidates,
self.test_candidates( start_block,
span, otherwise_block,
scrutinee_span, fake_borrows,
candidates, );
block, return;
otherwise_block,
fake_borrows,
);
return;
}
} }
let match_pairs = mem::take(&mut first_candidate.match_pairs); let match_pairs = mem::take(&mut first_candidate.match_pairs);
first_candidate.pre_binding_block = Some(block); let (first_match_pair, remaining_match_pairs) = match_pairs.split_first().unwrap();
let PatKind::Or { ref pats } = &first_match_pair.pattern.kind else { unreachable!() };
let remainder_start = self.cfg.start_new_block(); let remainder_start = self.cfg.start_new_block();
for match_pair in match_pairs { let or_span = first_match_pair.pattern.span;
let PatKind::Or { ref pats } = &match_pair.pattern.kind else { // Test the alternatives of this or-pattern.
bug!("Or-patterns should have been sorted to the end"); self.test_or_pattern(
}; first_candidate,
let or_span = match_pair.pattern.span; start_block,
remainder_start,
pats,
or_span,
&first_match_pair.place,
fake_borrows,
);
if !remaining_match_pairs.is_empty() {
// If more match pairs remain, test them after each subcandidate.
// We could add them to the or-candidates before the call to `test_or_pattern` but this
// would make it impossible to detect simplifiable or-patterns. That would guarantee
// exponentially large CFGs for cases like `(1 | 2, 3 | 4, ...)`.
first_candidate.visit_leaves(|leaf_candidate| { first_candidate.visit_leaves(|leaf_candidate| {
self.test_or_pattern( assert!(leaf_candidate.match_pairs.is_empty());
leaf_candidate, leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned());
remainder_start, let or_start = leaf_candidate.pre_binding_block.unwrap();
pats, // In a case like `(a | b, c | d)`, if `a` succeeds and `c | d` fails, we know `(b,
or_span, // c | d)` will fail too. If there is no guard, we skip testing of `b` by branching
&match_pair.place, // directly to `remainder_start`. If there is a guard, we have to try `(b, c | d)`.
let or_otherwise = leaf_candidate.otherwise_block.unwrap_or(remainder_start);
self.test_candidates_with_or(
span,
scrutinee_span,
&mut [leaf_candidate],
or_start,
or_otherwise,
fake_borrows, fake_borrows,
); );
}); });
} }
// Test the remaining candidates.
self.match_candidates( self.match_candidates(
span, span,
scrutinee_span, scrutinee_span,
@ -1460,17 +1475,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
otherwise_block, otherwise_block,
remaining_candidates, remaining_candidates,
fake_borrows, fake_borrows,
) );
} }
#[instrument( #[instrument(
skip(self, otherwise, or_span, place, fake_borrows, candidate, pats), skip(self, start_block, otherwise_block, or_span, place, fake_borrows, candidate, pats),
level = "debug" level = "debug"
)] )]
fn test_or_pattern<'pat>( fn test_or_pattern<'pat>(
&mut self, &mut self,
candidate: &mut Candidate<'pat, 'tcx>, candidate: &mut Candidate<'pat, 'tcx>,
otherwise: BasicBlock, start_block: BasicBlock,
otherwise_block: BasicBlock,
pats: &'pat [Box<Pat<'tcx>>], pats: &'pat [Box<Pat<'tcx>>],
or_span: Span, or_span: Span,
place: &PlaceBuilder<'tcx>, place: &PlaceBuilder<'tcx>,
@ -1482,16 +1498,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.map(|pat| Candidate::new(place.clone(), pat, candidate.has_guard, self)) .map(|pat| Candidate::new(place.clone(), pat, candidate.has_guard, self))
.collect(); .collect();
let mut or_candidate_refs: Vec<_> = or_candidates.iter_mut().collect(); let mut or_candidate_refs: Vec<_> = or_candidates.iter_mut().collect();
let otherwise = if let Some(otherwise_block) = candidate.otherwise_block {
otherwise_block
} else {
otherwise
};
self.match_candidates( self.match_candidates(
or_span, or_span,
or_span, or_span,
candidate.pre_binding_block.unwrap(), start_block,
otherwise, otherwise_block,
&mut or_candidate_refs, &mut or_candidate_refs,
fake_borrows, fake_borrows,
); );