From 670723e6fba094fa613082261ad89de387af7939 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 20 Jul 2024 22:18:35 +0200 Subject: [PATCH] Expand or-patterns as a separate step --- compiler/rustc_pattern_analysis/src/pat.rs | 13 +- .../rustc_pattern_analysis/src/usefulness.rs | 180 +++++++----------- 2 files changed, 86 insertions(+), 107 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 5e75976621e05..e8d6c542c5fd4 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -190,7 +190,18 @@ impl<'p, Cx: PatCx> PatOrWild<'p, Cx> { } } - /// Expand this (possibly-nested) or-pattern into its alternatives. + /// Expand this or-pattern into its alternatives. This only expands one or-pattern; use + /// `flatten_or_pat` to recursively expand nested or-patterns. + pub(crate) fn expand_or_pat(self) -> SmallVec<[Self; 1]> { + match self { + PatOrWild::Pat(pat) if pat.is_or_pat() => { + pat.iter_fields().map(|ipat| PatOrWild::Pat(&ipat.pat)).collect() + } + _ => smallvec![self], + } + } + + /// Recursively expand this (possibly-nested) or-pattern into its alternatives. pub(crate) fn flatten_or_pat(self) -> SmallVec<[Self; 1]> { match self { PatOrWild::Pat(pat) if pat.is_or_pat() => pat diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 6e96a93473f7d..8486792b5543f 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -462,8 +462,9 @@ //! # Or-patterns //! //! What we have described so far works well if there are no or-patterns. To handle them, if the -//! first pattern of a row in the matrix is an or-pattern, we expand it by duplicating the rest of -//! the row as necessary. This is handled automatically in [`Matrix`]. +//! first pattern of any row in the matrix is an or-pattern, we expand it by duplicating the rest of +//! the row as necessary. For code reuse, this is implemented as "specializing with the `Or` +//! constructor". //! //! This makes usefulness tracking subtle, because we also want to compute whether an alternative of //! an or-pattern is redundant, e.g. in `Some(_) | Some(0)`. We therefore track usefulness of each @@ -875,6 +876,11 @@ impl PlaceInfo { return Ok((smallvec![Constructor::PrivateUninhabited], vec![])); } + if ctors.clone().any(|c| matches!(c, Constructor::Or)) { + // If any constructor is `Or`, we expand or-patterns. + return Ok((smallvec![Constructor::Or], vec![])); + } + let ctors_for_ty = cx.ctors_for_ty(&self.ty)?; debug!(?ctors_for_ty); @@ -968,10 +974,6 @@ impl<'p, Cx: PatCx> PatStack<'p, Cx> { PatStack { pats: smallvec![PatOrWild::Pat(pat)], relevant: true } } - fn is_empty(&self) -> bool { - self.pats.is_empty() - } - fn len(&self) -> usize { self.pats.len() } @@ -984,10 +986,10 @@ impl<'p, Cx: PatCx> PatStack<'p, Cx> { self.pats.iter().copied() } - // Recursively expand the first or-pattern into its subpatterns. Only useful if the pattern is - // an or-pattern. Panics if `self` is empty. + // Expand the first or-pattern into its subpatterns. Only useful if the pattern is an + // or-pattern. Panics if `self` is empty. fn expand_or_pat(&self) -> impl Iterator> + Captures<'_> { - self.head().flatten_or_pat().into_iter().map(move |pat| { + self.head().expand_or_pat().into_iter().map(move |pat| { let mut new = self.clone(); new.pats[0] = pat; new @@ -1057,10 +1059,6 @@ struct MatrixRow<'p, Cx: PatCx> { } impl<'p, Cx: PatCx> MatrixRow<'p, Cx> { - fn is_empty(&self) -> bool { - self.pats.is_empty() - } - fn len(&self) -> usize { self.pats.len() } @@ -1073,12 +1071,14 @@ impl<'p, Cx: PatCx> MatrixRow<'p, Cx> { self.pats.iter() } - // Recursively expand the first or-pattern into its subpatterns. Only useful if the pattern is - // an or-pattern. Panics if `self` is empty. - fn expand_or_pat(&self) -> impl Iterator> + Captures<'_> { - self.pats.expand_or_pat().map(|patstack| MatrixRow { + // Expand the first or-pattern (if any) into its subpatterns. Panics if `self` is empty. + fn expand_or_pat( + &self, + parent_row: usize, + ) -> impl Iterator> + Captures<'_> { + self.pats.expand_or_pat().map(move |patstack| MatrixRow { pats: patstack, - parent_row: self.parent_row, + parent_row, is_under_guard: self.is_under_guard, useful: false, intersects: BitSet::new_empty(0), // Initialized in `Matrix::expand_and_push`. @@ -1100,7 +1100,7 @@ impl<'p, Cx: PatCx> MatrixRow<'p, Cx> { parent_row, is_under_guard: self.is_under_guard, useful: false, - intersects: BitSet::new_empty(0), // Initialized in `Matrix::expand_and_push`. + intersects: BitSet::new_empty(0), // Initialized in `Matrix::push`. }) } } @@ -1116,7 +1116,7 @@ impl<'p, Cx: PatCx> fmt::Debug for MatrixRow<'p, Cx> { /// Invariant: each row must have the same length, and each column must have the same type. /// /// Invariant: the first column must not contain or-patterns. This is handled by -/// [`Matrix::expand_and_push`]. +/// [`Matrix::push`]. /// /// In fact each column corresponds to a place inside the scrutinee of the match. E.g. after /// specializing `(,)` and `Some` on a pattern of type `(Option, bool)`, the first column of @@ -1136,19 +1136,10 @@ struct Matrix<'p, Cx: PatCx> { } impl<'p, Cx: PatCx> Matrix<'p, Cx> { - /// Pushes a new row to the matrix. If the row starts with an or-pattern, this recursively - /// expands it. Internal method, prefer [`Matrix::new`]. - fn expand_and_push(&mut self, mut row: MatrixRow<'p, Cx>) { - if !row.is_empty() && row.head().is_or_pat() { - // Expand nested or-patterns. - for mut new_row in row.expand_or_pat() { - new_row.intersects = BitSet::new_empty(self.rows.len()); - self.rows.push(new_row); - } - } else { - row.intersects = BitSet::new_empty(self.rows.len()); - self.rows.push(row); - } + /// Pushes a new row to the matrix. Internal method, prefer [`Matrix::new`]. + fn push(&mut self, mut row: MatrixRow<'p, Cx>) { + row.intersects = BitSet::new_empty(self.rows.len()); + self.rows.push(row); } /// Build a new matrix from an iterator of `MatchArm`s. @@ -1170,9 +1161,9 @@ impl<'p, Cx: PatCx> Matrix<'p, Cx> { parent_row: arm_id, is_under_guard: arm.has_guard, useful: false, - intersects: BitSet::new_empty(0), // Initialized in `Matrix::expand_and_push`. + intersects: BitSet::new_empty(0), // Initialized in `Matrix::push`. }; - matrix.expand_and_push(v); + matrix.push(v); } matrix } @@ -1209,22 +1200,38 @@ impl<'p, Cx: PatCx> Matrix<'p, Cx> { ctor: &Constructor, ctor_is_relevant: bool, ) -> Result, Cx::Error> { - let subfield_place_info = self.place_info[0].specialize(pcx.cx, ctor); - let arity = subfield_place_info.len(); - let specialized_place_info = - subfield_place_info.chain(self.place_info[1..].iter().cloned()).collect(); - let mut matrix = Matrix { - rows: Vec::new(), - place_info: specialized_place_info, - wildcard_row_is_relevant: self.wildcard_row_is_relevant && ctor_is_relevant, - }; - for (i, row) in self.rows().enumerate() { - if ctor.is_covered_by(pcx.cx, row.head().ctor())? { - let new_row = row.pop_head_constructor(pcx.cx, ctor, arity, ctor_is_relevant, i)?; - matrix.expand_and_push(new_row); + if matches!(ctor, Constructor::Or) { + // Specializing with `Or` means expanding rows with or-patterns. + let mut matrix = Matrix { + rows: Vec::new(), + place_info: self.place_info.clone(), + wildcard_row_is_relevant: self.wildcard_row_is_relevant, + }; + for (i, row) in self.rows().enumerate() { + for new_row in row.expand_or_pat(i) { + matrix.push(new_row); + } } + Ok(matrix) + } else { + let subfield_place_info = self.place_info[0].specialize(pcx.cx, ctor); + let arity = subfield_place_info.len(); + let specialized_place_info = + subfield_place_info.chain(self.place_info[1..].iter().cloned()).collect(); + let mut matrix = Matrix { + rows: Vec::new(), + place_info: specialized_place_info, + wildcard_row_is_relevant: self.wildcard_row_is_relevant && ctor_is_relevant, + }; + for (i, row) in self.rows().enumerate() { + if ctor.is_covered_by(pcx.cx, row.head().ctor())? { + let new_row = + row.pop_head_constructor(pcx.cx, ctor, arity, ctor_is_relevant, i)?; + matrix.push(new_row); + } + } + Ok(matrix) } - Ok(matrix) } /// Recover row usefulness and intersection information from a processed specialized matrix. @@ -1465,7 +1472,9 @@ impl WitnessMatrix { missing_ctors: &[Constructor], ctor: &Constructor, ) { - if self.is_empty() { + // The `Or` constructor indicates that we expanded or-patterns. This doesn't affect + // witnesses. + if self.is_empty() || matches!(ctor, Constructor::Or) { return; } if matches!(ctor, Constructor::Missing) { @@ -1715,48 +1724,6 @@ pub enum Usefulness<'p, Cx: PatCx> { Redundant, } -/// Report whether this pattern was found useful, and its subpatterns that were not useful if any. -fn collect_pattern_usefulness<'p, Cx: PatCx>( - useful_subpatterns: &FxHashSet, - pat: &'p DeconstructedPat, -) -> Usefulness<'p, Cx> { - fn pat_is_useful<'p, Cx: PatCx>( - useful_subpatterns: &FxHashSet, - pat: &'p DeconstructedPat, - ) -> bool { - if useful_subpatterns.contains(&pat.uid) { - true - } else if pat.is_or_pat() - && pat.iter_fields().any(|f| pat_is_useful(useful_subpatterns, &f.pat)) - { - // We always expand or patterns in the matrix, so we will never see the actual - // or-pattern (the one with constructor `Or`) in the column. As such, it will not be - // marked as useful itself, only its children will. We recover this information here. - true - } else { - false - } - } - - let mut redundant_subpats = Vec::new(); - pat.walk(&mut |p| { - if pat_is_useful(useful_subpatterns, p) { - // The pattern is useful, so we recurse to find redundant subpatterns. - true - } else { - // The pattern is redundant. - redundant_subpats.push(p); - false // stop recursing - } - }); - - if pat_is_useful(useful_subpatterns, pat) { - Usefulness::Useful(redundant_subpats) - } else { - Usefulness::Redundant - } -} - /// The output of checking a match for exhaustiveness and arm usefulness. pub struct UsefulnessReport<'p, Cx: PatCx> { /// For each arm of the input, whether that arm is useful after the arms above it. @@ -1793,25 +1760,26 @@ pub fn compute_match_usefulness<'p, Cx: PatCx>( .copied() .map(|arm| { debug!(?arm); - let usefulness = collect_pattern_usefulness(&cx.useful_subpatterns, arm.pat); + let usefulness = if cx.useful_subpatterns.contains(&arm.pat.uid) { + let mut redundant_subpats = Vec::new(); + arm.pat.walk(&mut |subpat| { + if cx.useful_subpatterns.contains(&subpat.uid) { + true // keep recursing + } else { + redundant_subpats.push(subpat); + false // stop recursing + } + }); + Usefulness::Useful(redundant_subpats) + } else { + Usefulness::Redundant + }; debug!(?usefulness); (arm, usefulness) }) .collect(); - let mut arm_intersections: Vec<_> = - arms.iter().enumerate().map(|(i, _)| BitSet::new_empty(i)).collect(); - for row in matrix.rows() { - let arm_id = row.parent_row; - for intersection in row.intersects.iter() { - // Convert the matrix row ids into arm ids (they can differ because we expand or-patterns). - let arm_intersection = matrix.rows[intersection].parent_row; - // Note: self-intersection can happen with or-patterns. - if arm_intersection != arm_id { - arm_intersections[arm_id].insert(arm_intersection); - } - } - } + let arm_intersections: Vec<_> = matrix.rows().map(|row| row.intersects.clone()).collect(); Ok(UsefulnessReport { arm_usefulness, non_exhaustiveness_witnesses, arm_intersections }) }