Skip to content

Commit

Permalink
Fix a subtle regression
Browse files Browse the repository at this point in the history
Before, the SwitchInt cases were computed in two passes: if the first
pass accepted e.g. 0..=5 and then 1, the second pass would not accept
0..=5 anymore because 1 would be listed in the SwitchInt options.

Now there's a single pass, so if we sort 0..=5 we must take care to not
sort a subsequent 1.
  • Loading branch information
Nadrieril committed Mar 2, 2024
1 parent edea739 commit d46ff64
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 3 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,12 @@ enum TestCase<'pat, 'tcx> {
Or { pats: Box<[FlatPat<'pat, 'tcx>]> },
}

impl<'pat, 'tcx> TestCase<'pat, 'tcx> {
fn as_range(&self) -> Option<&'pat PatRange<'tcx>> {
if let Self::Range(v) = self { Some(*v) } else { None }
}
}

#[derive(Debug, Clone)]
pub(crate) struct MatchPair<'pat, 'tcx> {
/// This place...
Expand Down
27 changes: 24 additions & 3 deletions compiler/rustc_mir_build/src/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,9 +510,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
(TestKind::SwitchInt, &TestCase::Constant { value })
if is_switch_ty(match_pair.pattern.ty) =>
{
fully_matched = true;
let bits = value.eval_bits(self.tcx, self.param_env);
Some(TestBranch::Constant(value, bits))
// Beware: there might be some ranges sorted into the failure case; we must not add
// a success case that could be matched by one of these ranges.
let is_covering_range = |test_case: &TestCase<'_, 'tcx>| {
test_case.as_range().is_some_and(|range| {
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 == *test_place && is_covering_range(&mp.test_case))
};
if sorted_candidates
.get(&TestBranch::Failure)
.is_some_and(|candidates| candidates.iter().any(is_conflicting_candidate))
{
fully_matched = false;
None
} else {
fully_matched = true;
let bits = value.eval_bits(self.tcx, self.param_env);
Some(TestBranch::Constant(value, bits))
}
}
(TestKind::SwitchInt, TestCase::Range(range)) => {
fully_matched = false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@ run-pass
//
// Regression test for match lowering to MIR: when gathering candidates, by the time we get to the
// range we know the range will only match on the failure case of the switchint. Hence we mustn't
// add the `1` to the switchint or the range would be incorrectly sorted.
#![allow(unreachable_patterns)]
fn main() {
match 1 {
10 => unreachable!(),
0..=5 => {}
1 => unreachable!(),
_ => unreachable!(),
}
}

0 comments on commit d46ff64

Please sign in to comment.