Skip to content

Commit 22b8a7d

Browse files
NadrierilNadrieril
committed
Improve or-pattern simplification
We can now tell ahead of time whether and or-pattern will be simplifiable or not. We use this to clarify the possible code paths.
1 parent 0ad2eff commit 22b8a7d

File tree

1 file changed

+87
-86
lines changed
  • compiler/rustc_mir_build/src/build/matches

1 file changed

+87
-86
lines changed

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

Lines changed: 87 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,8 +1398,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
13981398
/// Tests a candidate where there are only or-patterns left to test, or
13991399
/// forwards to [Builder::test_candidates].
14001400
///
1401-
/// Given a pattern `(P | Q, R | S)` we (in principle) generate a CFG like
1402-
/// so:
1401+
/// Given a pattern `(P | Q, R | S)` we would like to generate a CFG like so:
1402+
///
1403+
/// ```text
1404+
/// ...
1405+
/// +---------------+------------+
1406+
/// | | |
1407+
/// [ P matches ] [ Q matches ] [ otherwise ]
1408+
/// | | |
1409+
/// +---------------+ |
1410+
/// | ...
1411+
/// [ match R, S ]
1412+
/// |
1413+
/// ...
1414+
/// ```
1415+
///
1416+
/// In practice there are some complications:
1417+
///
1418+
/// * If `P` or `Q` has bindings or type ascriptions, we must generate separate branches for
1419+
/// each case.
1420+
/// * If there's a guard, we must also keep branches separate as this changes how many times
1421+
/// the guard is run.
1422+
/// * If `P` succeeds and `R | S` fails, we know `(Q, R | S)` will fail too. So we could skip
1423+
/// testing of `Q` in that case. Because we can't distinguish pattern failure from guard
1424+
/// failure, we only do this optimization when there is no guard. We then get this:
14031425
///
14041426
/// ```text
14051427
/// [ start ]
@@ -1427,27 +1449,36 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
14271449
/// [ Success ] [ Failure ]
14281450
/// ```
14291451
///
1430-
/// In practice there are some complications:
1431-
///
1432-
/// * If there's a guard, then the otherwise branch of the first match on
1433-
/// `R | S` goes to a test for whether `Q` matches, and the control flow
1434-
/// doesn't merge into a single success block until after the guard is
1435-
/// tested.
1436-
/// * If neither `P` or `Q` has any bindings or type ascriptions and there
1437-
/// isn't a match guard, then we create a smaller CFG like:
1452+
/// * If there's a guard, then the otherwise branch of the first match on `R | S` goes to a test
1453+
/// for whether `Q` matches, and the control flow doesn't merge into a single success block
1454+
/// until after the guard is tested. In other words, the branches are kept entirely separate:
14381455
///
14391456
/// ```text
1440-
/// ...
1441-
/// +---------------+------------+
1442-
/// | | |
1443-
/// [ P matches ] [ Q matches ] [ otherwise ]
1444-
/// | | |
1445-
/// +---------------+ |
1446-
/// | ...
1447-
/// [ match R, S ]
1457+
/// [ start ]
14481458
/// |
1449-
/// ...
1459+
/// [ match P, Q ]
1460+
/// |
1461+
/// +---------------------------+-------------+------------------------------------+
1462+
/// | | | |
1463+
/// V | V V
1464+
/// [ P matches ] | [ Q matches ] [ otherwise ]
1465+
/// | | | |
1466+
/// V [ otherwise ] V |
1467+
/// [ match R, S ] ^ [ match R, S ] |
1468+
/// | | | |
1469+
/// +--------------+------------+ +--------------+------------+ |
1470+
/// | | | | | |
1471+
/// V V V V V |
1472+
/// [ R matches ] [ S matches ] [ R matches ] [ S matches ] [otherwise ] |
1473+
/// | | | | | |
1474+
/// +--------------+--------------------------+--------------+ | |
1475+
/// | | |
1476+
/// | +--------+
1477+
/// | |
1478+
/// V V
1479+
/// [ Success ] [ Failure ]
14501480
/// ```
1481+
///
14511482
fn test_candidates_with_or(
14521483
&mut self,
14531484
span: Span,
@@ -1465,25 +1496,43 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
14651496

14661497
let first_match_pair = first_candidate.match_pairs.remove(0);
14671498
let or_span = first_match_pair.pattern.span;
1468-
let TestCase::Or { pats, .. } = first_match_pair.test_case else { unreachable!() };
1499+
let TestCase::Or { pats, simple } = first_match_pair.test_case else { unreachable!() };
1500+
debug!("candidate={:#?}\npats={:#?}", first_candidate, pats);
14691501

1470-
let remainder_start = self.cfg.start_new_block();
14711502
// Test the alternatives of this or-pattern.
1472-
self.test_or_pattern(first_candidate, start_block, remainder_start, pats, or_span);
1503+
let remainder_start = self.cfg.start_new_block();
1504+
first_candidate.subcandidates = pats
1505+
.into_vec()
1506+
.into_iter()
1507+
.map(|flat_pat| Candidate::from_flat_pat(flat_pat, first_candidate.has_guard))
1508+
.collect();
1509+
let mut or_candidate_refs: Vec<_> = first_candidate.subcandidates.iter_mut().collect();
1510+
self.match_candidates(
1511+
or_span,
1512+
or_span,
1513+
start_block,
1514+
remainder_start,
1515+
&mut or_candidate_refs,
1516+
);
1517+
1518+
// Whether we need to keep the or-pattern branches separate.
1519+
let keep_branches_separate = first_candidate.has_guard || !simple;
1520+
if !keep_branches_separate {
1521+
self.merge_subcandidates(first_candidate, self.source_info(or_span));
1522+
}
14731523

14741524
if !first_candidate.match_pairs.is_empty() {
1475-
// If more match pairs remain, test them after each subcandidate.
1476-
// We could add them to the or-candidates before the call to `test_or_pattern` but this
1477-
// would make it impossible to detect simplifiable or-patterns. That would guarantee
1478-
// exponentially large CFGs for cases like `(1 | 2, 3 | 4, ...)`.
1525+
// If more match pairs remain, test them after each subcandidate. If we merged them,
1526+
// then this will only test `first_candidate`.
14791527
let remaining_match_pairs = mem::take(&mut first_candidate.match_pairs);
14801528
first_candidate.visit_leaves(|leaf_candidate| {
14811529
assert!(leaf_candidate.match_pairs.is_empty());
14821530
leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned());
14831531
let or_start = leaf_candidate.pre_binding_block.unwrap();
1484-
// In a case like `(a | b, c | d)`, if `a` succeeds and `c | d` fails, we know `(b,
1485-
// c | d)` will fail too. If there is no guard, we skip testing of `b` by branching
1486-
// directly to `remainder_start`. If there is a guard, we have to try `(b, c | d)`.
1532+
// In a case like `(P | Q, R | S)`, if `P` succeeds and `R | S` fails, we know `(Q,
1533+
// R | S)` will fail too. If there is no guard, we skip testing of `Q` by branching
1534+
// directly to `remainder_start`. If there is a guard, `or_otherwise` can be reached
1535+
// by guard failure as well, so we can't skip `Q`.
14871536
let or_otherwise = leaf_candidate.otherwise_block.unwrap_or(remainder_start);
14881537
self.test_candidates_with_or(
14891538
span,
@@ -1505,68 +1554,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
15051554
);
15061555
}
15071556

1508-
#[instrument(
1509-
skip(self, start_block, otherwise_block, or_span, candidate, pats),
1510-
level = "debug"
1511-
)]
1512-
fn test_or_pattern<'pat>(
1513-
&mut self,
1514-
candidate: &mut Candidate<'pat, 'tcx>,
1515-
start_block: BasicBlock,
1516-
otherwise_block: BasicBlock,
1517-
pats: Box<[FlatPat<'pat, 'tcx>]>,
1518-
or_span: Span,
1519-
) {
1520-
debug!("candidate={:#?}\npats={:#?}", candidate, pats);
1521-
let mut or_candidates: Vec<_> = pats
1522-
.into_vec()
1523-
.into_iter()
1524-
.map(|flat_pat| Candidate::from_flat_pat(flat_pat, candidate.has_guard))
1525-
.collect();
1526-
let mut or_candidate_refs: Vec<_> = or_candidates.iter_mut().collect();
1527-
self.match_candidates(
1528-
or_span,
1529-
or_span,
1530-
start_block,
1531-
otherwise_block,
1532-
&mut or_candidate_refs,
1533-
);
1534-
candidate.subcandidates = or_candidates;
1535-
self.merge_trivial_subcandidates(candidate, self.source_info(or_span));
1536-
}
1537-
1538-
/// Try to merge all of the subcandidates of the given candidate into one.
1557+
/// Merge all of the subcandidates of the given candidate into one.
15391558
/// This avoids exponentially large CFGs in cases like `(1 | 2, 3 | 4, ...)`.
1540-
fn merge_trivial_subcandidates(
1559+
fn merge_subcandidates(
15411560
&mut self,
15421561
candidate: &mut Candidate<'_, 'tcx>,
15431562
source_info: SourceInfo,
15441563
) {
1545-
if candidate.subcandidates.is_empty() || candidate.has_guard {
1546-
// FIXME(or_patterns; matthewjasper) Don't give up if we have a guard.
1547-
return;
1548-
}
1549-
1550-
let mut can_merge = true;
1551-
1552-
// Not `Iterator::all` because we don't want to short-circuit.
1553-
for subcandidate in &mut candidate.subcandidates {
1554-
self.merge_trivial_subcandidates(subcandidate, source_info);
1555-
1556-
// FIXME(or_patterns; matthewjasper) Try to be more aggressive here.
1557-
can_merge &= subcandidate.subcandidates.is_empty()
1558-
&& subcandidate.bindings.is_empty()
1559-
&& subcandidate.ascriptions.is_empty();
1560-
}
1561-
1562-
if can_merge {
1563-
let any_matches = self.cfg.start_new_block();
1564-
for subcandidate in mem::take(&mut candidate.subcandidates) {
1565-
let or_block = subcandidate.pre_binding_block.unwrap();
1566-
self.cfg.goto(or_block, source_info, any_matches);
1567-
}
1568-
candidate.pre_binding_block = Some(any_matches);
1569-
}
1564+
let any_matches = self.cfg.start_new_block();
1565+
candidate.visit_leaves(|leaf_candidate| {
1566+
let or_block = leaf_candidate.pre_binding_block.unwrap();
1567+
self.cfg.goto(or_block, source_info, any_matches);
1568+
});
1569+
candidate.subcandidates.clear();
1570+
candidate.pre_binding_block = Some(any_matches);
15701571
}
15711572

15721573
/// Pick a test to run. Which test doesn't matter as long as it is guaranteed to fully match at

0 commit comments

Comments
 (0)