Skip to content

Commit 4daa43a

Browse files
authored
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`
2 parents 082b97a + c1514a6 commit 4daa43a

File tree

1 file changed

+51
-40
lines changed
  • compiler/rustc_mir_build/src/build/matches

1 file changed

+51
-40
lines changed

compiler/rustc_mir_build/src/build/matches/mod.rs

+51-40
Original file line numberDiff line numberDiff line change
@@ -1052,7 +1052,7 @@ struct Ascription<'tcx> {
10521052
variance: ty::Variance,
10531053
}
10541054

1055-
#[derive(Debug)]
1055+
#[derive(Debug, Clone)]
10561056
pub(crate) struct MatchPair<'pat, 'tcx> {
10571057
// This place...
10581058
place: PlaceBuilder<'tcx>,
@@ -1408,69 +1408,85 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
14081408
span: Span,
14091409
scrutinee_span: Span,
14101410
candidates: &mut [&mut Candidate<'_, 'tcx>],
1411-
block: BasicBlock,
1411+
start_block: BasicBlock,
14121412
otherwise_block: BasicBlock,
14131413
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
14141414
) {
14151415
let (first_candidate, remaining_candidates) = candidates.split_first_mut().unwrap();
1416-
1417-
// All of the or-patterns have been sorted to the end, so if the first
1418-
// pattern is an or-pattern we only have or-patterns.
1419-
match first_candidate.match_pairs[0].pattern.kind {
1420-
PatKind::Or { .. } => (),
1421-
_ => {
1422-
self.test_candidates(
1423-
span,
1424-
scrutinee_span,
1425-
candidates,
1426-
block,
1427-
otherwise_block,
1428-
fake_borrows,
1429-
);
1430-
return;
1431-
}
1416+
assert!(first_candidate.subcandidates.is_empty());
1417+
if !matches!(first_candidate.match_pairs[0].pattern.kind, PatKind::Or { .. }) {
1418+
self.test_candidates(
1419+
span,
1420+
scrutinee_span,
1421+
candidates,
1422+
start_block,
1423+
otherwise_block,
1424+
fake_borrows,
1425+
);
1426+
return;
14321427
}
14331428

14341429
let match_pairs = mem::take(&mut first_candidate.match_pairs);
1435-
first_candidate.pre_binding_block = Some(block);
1430+
let (first_match_pair, remaining_match_pairs) = match_pairs.split_first().unwrap();
1431+
let PatKind::Or { ref pats } = &first_match_pair.pattern.kind else { unreachable!() };
14361432

14371433
let remainder_start = self.cfg.start_new_block();
1438-
for match_pair in match_pairs {
1439-
let PatKind::Or { ref pats } = &match_pair.pattern.kind else {
1440-
bug!("Or-patterns should have been sorted to the end");
1441-
};
1442-
let or_span = match_pair.pattern.span;
1434+
let or_span = first_match_pair.pattern.span;
1435+
// Test the alternatives of this or-pattern.
1436+
self.test_or_pattern(
1437+
first_candidate,
1438+
start_block,
1439+
remainder_start,
1440+
pats,
1441+
or_span,
1442+
&first_match_pair.place,
1443+
fake_borrows,
1444+
);
14431445

1446+
if !remaining_match_pairs.is_empty() {
1447+
// If more match pairs remain, test them after each subcandidate.
1448+
// We could add them to the or-candidates before the call to `test_or_pattern` but this
1449+
// would make it impossible to detect simplifiable or-patterns. That would guarantee
1450+
// exponentially large CFGs for cases like `(1 | 2, 3 | 4, ...)`.
14441451
first_candidate.visit_leaves(|leaf_candidate| {
1445-
self.test_or_pattern(
1446-
leaf_candidate,
1447-
remainder_start,
1448-
pats,
1449-
or_span,
1450-
&match_pair.place,
1452+
assert!(leaf_candidate.match_pairs.is_empty());
1453+
leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned());
1454+
let or_start = leaf_candidate.pre_binding_block.unwrap();
1455+
// In a case like `(a | b, c | d)`, if `a` succeeds and `c | d` fails, we know `(b,
1456+
// c | d)` will fail too. If there is no guard, we skip testing of `b` by branching
1457+
// directly to `remainder_start`. If there is a guard, we have to try `(b, c | d)`.
1458+
let or_otherwise = leaf_candidate.otherwise_block.unwrap_or(remainder_start);
1459+
self.test_candidates_with_or(
1460+
span,
1461+
scrutinee_span,
1462+
&mut [leaf_candidate],
1463+
or_start,
1464+
or_otherwise,
14511465
fake_borrows,
14521466
);
14531467
});
14541468
}
14551469

1470+
// Test the remaining candidates.
14561471
self.match_candidates(
14571472
span,
14581473
scrutinee_span,
14591474
remainder_start,
14601475
otherwise_block,
14611476
remaining_candidates,
14621477
fake_borrows,
1463-
)
1478+
);
14641479
}
14651480

14661481
#[instrument(
1467-
skip(self, otherwise, or_span, place, fake_borrows, candidate, pats),
1482+
skip(self, start_block, otherwise_block, or_span, place, fake_borrows, candidate, pats),
14681483
level = "debug"
14691484
)]
14701485
fn test_or_pattern<'pat>(
14711486
&mut self,
14721487
candidate: &mut Candidate<'pat, 'tcx>,
1473-
otherwise: BasicBlock,
1488+
start_block: BasicBlock,
1489+
otherwise_block: BasicBlock,
14741490
pats: &'pat [Box<Pat<'tcx>>],
14751491
or_span: Span,
14761492
place: &PlaceBuilder<'tcx>,
@@ -1482,16 +1498,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
14821498
.map(|pat| Candidate::new(place.clone(), pat, candidate.has_guard, self))
14831499
.collect();
14841500
let mut or_candidate_refs: Vec<_> = or_candidates.iter_mut().collect();
1485-
let otherwise = if let Some(otherwise_block) = candidate.otherwise_block {
1486-
otherwise_block
1487-
} else {
1488-
otherwise
1489-
};
14901501
self.match_candidates(
14911502
or_span,
14921503
or_span,
1493-
candidate.pre_binding_block.unwrap(),
1494-
otherwise,
1504+
start_block,
1505+
otherwise_block,
14951506
&mut or_candidate_refs,
14961507
fake_borrows,
14971508
);

0 commit comments

Comments
 (0)