From d46ff6415c033ccfebac3d2a757908611a67d324 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 2 Mar 2024 02:49:33 +0100 Subject: [PATCH] Fix a subtle regression 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. --- .../rustc_mir_build/src/build/matches/mod.rs | 6 +++++ .../rustc_mir_build/src/build/matches/test.rs | 27 ++++++++++++++++--- ...egression-switchint-sorting-with-ranges.rs | 14 ++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 tests/ui/pattern/usefulness/integer-ranges/regression-switchint-sorting-with-ranges.rs diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index fdd41eeabecc0..cfd12cec0e468 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -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... diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 0598ffccea7b7..9d77bd063e114 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -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; diff --git a/tests/ui/pattern/usefulness/integer-ranges/regression-switchint-sorting-with-ranges.rs b/tests/ui/pattern/usefulness/integer-ranges/regression-switchint-sorting-with-ranges.rs new file mode 100644 index 0000000000000..bacb60a108bfc --- /dev/null +++ b/tests/ui/pattern/usefulness/integer-ranges/regression-switchint-sorting-with-ranges.rs @@ -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!(), + } +}