diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs index 637cc38be2c7c..7b8a45f65ed96 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs @@ -299,6 +299,74 @@ //! //! //! +//! # `Missing` and relevant constructors +//! +//! Take the following example: +//! +//! ```compile_fail,E0004 +//! enum Direction { North, South, East, West } +//! # let wind = (Direction::North, 0u8); +//! match wind { +//! (Direction::North, _) => {} // arm 1 +//! (_, 50..) => {} // arm 2 +//! } +//! ``` +//! +//! Remember that we represent the "everything else" cases with [`Constructor::Missing`]. When we +//! specialize with `Missing` in the first column, we have one arm left: +//! +//! ```ignore(partial code) +//! (50..) => {} // arm 2 +//! ``` +//! +//! We then conclude that arm 2 is useful, and that the match is non-exhaustive with witness +//! `(Missing, 0..50)` (which we would display to the user as `(_, 0..50)`). +//! +//! When we then specialize with `North`, we have two arms left: +//! +//! ```ignore(partial code) +//! (_) => {} // arm 1 +//! (50..) => {} // arm 2 +//! ``` +//! +//! Because `Missing` only matches wildcard rows, specializing with `Missing` is guaranteed to +//! result in a subset of the rows obtained from specializing with anything else. This means that +//! any row with a wildcard found useful when specializing with anything else would also be found +//! useful in the `Missing` case. In our example, after specializing with `North` here we will not +//! gain new information regarding the usefulness of arm 2 or of the fake wildcard row used for +//! exhaustiveness. This allows us to skip cases. +//! +//! When specializing, if there is a `Missing` case we call the other constructors "irrelevant". +//! When there is no `Missing` case there are no irrelevant constructors. +//! +//! What happens then is: when we specialize a wildcard with an irrelevant constructor, we know we +//! won't get new info for this row; we consider that row "irrelevant". Whenever all the rows are +//! found irrelevant, we can safely skip the case entirely. +//! +//! In the example above, we will entirely skip the `(North, 50..)` case. This skipping was +//! developped as a solution to #118437. It doesn't look like much but it can save us from +//! exponential blowup. +//! +//! There's a subtlety regarding exhaustiveness: while this shortcutting doesn't affect correctness, +//! it can affect which witnesses are reported. For example, in the following: +//! +//! ```compile_fail,E0004 +//! # let foo = (true, true, true); +//! match foo { +//! (true, _, true) => {} +//! (_, true, _) => {} +//! } +//! ``` +//! +//! In this example we will skip the `(true, true, _)` case entirely. Thus `(true, true, false)` +//! will not be reported as missing. In fact we go further than this: we deliberately do not report +//! any cases that are irrelevant for the fake wildcard row. For example, in `match ... { (true, +//! true) => {} }` we will not report `(true, false)` as missing. This was a deliberate choice made +//! early in the development of rust; it so happens that it is beneficial for performance reasons +//! too. +//! +//! +//! //! # Or-patterns //! //! What we have described so far works well if there are no or-patterns. To handle them, if the @@ -710,11 +778,15 @@ impl fmt::Display for ValidityConstraint { struct PatStack<'p, 'tcx> { // Rows of len 1 are very common, which is why `SmallVec[_; 2]` works well. pats: SmallVec<[&'p DeconstructedPat<'p, 'tcx>; 2]>, + /// Sometimes we know that as far as this row is concerned, the current case is already handled + /// by a different, more general, case. When all rows are irrelevant this allows us to skip many + /// branches. This is purely an optimization. See at the top for details. + relevant: bool, } impl<'p, 'tcx> PatStack<'p, 'tcx> { fn from_pattern(pat: &'p DeconstructedPat<'p, 'tcx>) -> Self { - PatStack { pats: smallvec![pat] } + PatStack { pats: smallvec![pat], relevant: true } } fn is_empty(&self) -> bool { @@ -749,12 +821,17 @@ impl<'p, 'tcx> PatStack<'p, 'tcx> { &self, pcx: &PatCtxt<'_, 'p, 'tcx>, ctor: &Constructor<'tcx>, + ctor_is_relevant: bool, ) -> PatStack<'p, 'tcx> { // We pop the head pattern and push the new fields extracted from the arguments of // `self.head()`. let mut new_pats = self.head().specialize(pcx, ctor); new_pats.extend_from_slice(&self.pats[1..]); - PatStack { pats: new_pats } + // `ctor` is relevant for this row if it is the actual constructor of this row, or if the + // row has a wildcard and `ctor` is relevant for wildcards. + let ctor_is_relevant = + !matches!(self.head().ctor(), Constructor::Wildcard) || ctor_is_relevant; + PatStack { pats: new_pats, relevant: self.relevant && ctor_is_relevant } } } @@ -820,10 +897,11 @@ impl<'p, 'tcx> MatrixRow<'p, 'tcx> { &self, pcx: &PatCtxt<'_, 'p, 'tcx>, ctor: &Constructor<'tcx>, + ctor_is_relevant: bool, parent_row: usize, ) -> MatrixRow<'p, 'tcx> { MatrixRow { - pats: self.pats.pop_head_constructor(pcx, ctor), + pats: self.pats.pop_head_constructor(pcx, ctor, ctor_is_relevant), parent_row, is_under_guard: self.is_under_guard, useful: false, @@ -952,8 +1030,9 @@ impl<'p, 'tcx> Matrix<'p, 'tcx> { &self, pcx: &PatCtxt<'_, 'p, 'tcx>, ctor: &Constructor<'tcx>, + ctor_is_relevant: bool, ) -> Matrix<'p, 'tcx> { - let wildcard_row = self.wildcard_row.pop_head_constructor(pcx, ctor); + let wildcard_row = self.wildcard_row.pop_head_constructor(pcx, ctor, ctor_is_relevant); let new_validity = self.place_validity[0].specialize(pcx, ctor); let new_place_validity = std::iter::repeat(new_validity) .take(ctor.arity(pcx)) @@ -963,7 +1042,7 @@ impl<'p, 'tcx> Matrix<'p, 'tcx> { Matrix { rows: Vec::new(), wildcard_row, place_validity: new_place_validity }; for (i, row) in self.rows().enumerate() { if ctor.is_covered_by(pcx, row.head().ctor()) { - let new_row = row.pop_head_constructor(pcx, ctor, i); + let new_row = row.pop_head_constructor(pcx, ctor, ctor_is_relevant, i); matrix.expand_and_push(new_row); } } @@ -1161,7 +1240,10 @@ impl<'tcx> WitnessMatrix<'tcx> { if matches!(ctor, Constructor::Missing) { // We got the special `Missing` constructor that stands for the constructors not present // in the match. - if !report_individual_missing_ctors { + if missing_ctors.is_empty() { + // Nothing to report. + *self = Self::empty(); + } else if !report_individual_missing_ctors { // Report `_` as missing. let pat = WitnessPat::wild_from_ctor(pcx, Constructor::Wildcard); self.push_pattern(pat); @@ -1220,6 +1302,15 @@ fn compute_exhaustiveness_and_usefulness<'p, 'tcx>( ) -> WitnessMatrix<'tcx> { debug_assert!(matrix.rows().all(|r| r.len() == matrix.column_count())); + if !matrix.wildcard_row.relevant && matrix.rows().all(|r| !r.pats.relevant) { + // Here we know that nothing will contribute further to exhaustiveness or usefulness. This + // is purely an optimization: skipping this check doesn't affect correctness. This check + // does change runtime behavior from exponential to quadratic on some matches found in the + // wild, so it's pretty important. It also affects which missing patterns will be reported. + // See the top of the file for details. + return WitnessMatrix::empty(); + } + let Some(ty) = matrix.head_ty() else { // The base case: there are no columns in the matrix. We are morally pattern-matching on (). // A row is useful iff it has no (unguarded) rows above it. @@ -1232,8 +1323,14 @@ fn compute_exhaustiveness_and_usefulness<'p, 'tcx>( return WitnessMatrix::empty(); } } - // No (unguarded) rows, so the match is not exhaustive. We return a new witness. - return WitnessMatrix::unit_witness(); + // No (unguarded) rows, so the match is not exhaustive. We return a new witness unless + // irrelevant. + return if matrix.wildcard_row.relevant { + WitnessMatrix::unit_witness() + } else { + // We can omit the witness without affecting correctness, so we do. + WitnessMatrix::empty() + }; }; debug!("ty: {ty:?}"); @@ -1274,32 +1371,21 @@ fn compute_exhaustiveness_and_usefulness<'p, 'tcx>( let mut ret = WitnessMatrix::empty(); for ctor in split_ctors { - debug!("specialize({:?})", ctor); // Dig into rows that match `ctor`. - let mut spec_matrix = matrix.specialize_constructor(pcx, &ctor); + debug!("specialize({:?})", ctor); + // `ctor` is *irrelevant* if there's another constructor in `split_ctors` that matches + // strictly fewer rows. In that case we can sometimes skip it. See the top of the file for + // details. + let ctor_is_relevant = matches!(ctor, Constructor::Missing) || missing_ctors.is_empty(); + let mut spec_matrix = matrix.specialize_constructor(pcx, &ctor, ctor_is_relevant); let mut witnesses = ensure_sufficient_stack(|| { compute_exhaustiveness_and_usefulness(cx, &mut spec_matrix, false) }); - let counts_for_exhaustiveness = match ctor { - Constructor::Missing => !missing_ctors.is_empty(), - // If there are missing constructors we'll report those instead. Since `Missing` matches - // only the wildcard rows, it matches fewer rows than this constructor, and is therefore - // guaranteed to result in the same or more witnesses. So skipping this does not - // jeopardize correctness. - _ => missing_ctors.is_empty(), - }; - if counts_for_exhaustiveness { - // Transform witnesses for `spec_matrix` into witnesses for `matrix`. - witnesses.apply_constructor( - pcx, - &missing_ctors, - &ctor, - report_individual_missing_ctors, - ); - // Accumulate the found witnesses. - ret.extend(witnesses); - } + // Transform witnesses for `spec_matrix` into witnesses for `matrix`. + witnesses.apply_constructor(pcx, &missing_ctors, &ctor, report_individual_missing_ctors); + // Accumulate the found witnesses. + ret.extend(witnesses); // A parent row is useful if any of its children is. for child_row in spec_matrix.rows() { diff --git a/tests/ui/pattern/usefulness/issue-118437-exponential-time-on-diagonal-match.rs b/tests/ui/pattern/usefulness/issue-118437-exponential-time-on-diagonal-match.rs new file mode 100644 index 0000000000000..39ad2d4abf3cc --- /dev/null +++ b/tests/ui/pattern/usefulness/issue-118437-exponential-time-on-diagonal-match.rs @@ -0,0 +1,72 @@ +// check-pass +struct BaseCommand { + field01: bool, + field02: bool, + field03: bool, + field04: bool, + field05: bool, + field06: bool, + field07: bool, + field08: bool, + field09: bool, + field10: bool, + field11: bool, + field12: bool, + field13: bool, + field14: bool, + field15: bool, + field16: bool, + field17: bool, + field18: bool, + field19: bool, + field20: bool, + field21: bool, + field22: bool, + field23: bool, + field24: bool, + field25: bool, + field26: bool, + field27: bool, + field28: bool, + field29: bool, + field30: bool, +} + +fn request_key(command: BaseCommand) { + match command { + BaseCommand { field01: true, .. } => {} + BaseCommand { field02: true, .. } => {} + BaseCommand { field03: true, .. } => {} + BaseCommand { field04: true, .. } => {} + BaseCommand { field05: true, .. } => {} + BaseCommand { field06: true, .. } => {} + BaseCommand { field07: true, .. } => {} + BaseCommand { field08: true, .. } => {} + BaseCommand { field09: true, .. } => {} + BaseCommand { field10: true, .. } => {} + BaseCommand { field11: true, .. } => {} + BaseCommand { field12: true, .. } => {} + BaseCommand { field13: true, .. } => {} + BaseCommand { field14: true, .. } => {} + BaseCommand { field15: true, .. } => {} + BaseCommand { field16: true, .. } => {} + BaseCommand { field17: true, .. } => {} + BaseCommand { field18: true, .. } => {} + BaseCommand { field19: true, .. } => {} + BaseCommand { field20: true, .. } => {} + BaseCommand { field21: true, .. } => {} + BaseCommand { field22: true, .. } => {} + BaseCommand { field23: true, .. } => {} + BaseCommand { field24: true, .. } => {} + BaseCommand { field25: true, .. } => {} + BaseCommand { field26: true, .. } => {} + BaseCommand { field27: true, .. } => {} + BaseCommand { field28: true, .. } => {} + BaseCommand { field29: true, .. } => {} + BaseCommand { field30: true, .. } => {} + + _ => {} + } +} + +fn main() {}