diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index 97edbd83b89ce..2c37f7bdb45d8 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -402,7 +402,7 @@ fn report_arm_reachability<'p, 'tcx>( Useful(unreachables) if unreachables.is_empty() => {} // The arm is reachable, but contains unreachable subpatterns (from or-patterns). Useful(unreachables) => { - let mut unreachables: Vec<_> = unreachables.iter().flatten().copied().collect(); + let mut unreachables: Vec<_> = unreachables.iter().collect(); // Emit lints in the order in which they occur in the file. unreachables.sort_unstable(); for span in unreachables { diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs index f3e1507b37ae1..0ecc034ac0ba9 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs @@ -311,7 +311,6 @@ use super::{Pat, PatKind}; use super::{PatternFoldable, PatternFolder}; use rustc_data_structures::captures::Captures; -use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::OnceCell; use rustc_arena::TypedArena; @@ -626,11 +625,82 @@ impl<'p, 'tcx> FromIterator> for Matrix<'p, 'tcx> { } } +/// Represents a set of `Span`s closed under the containment relation. That is, if a `Span` is +/// contained in the set then all `Span`s contained in it are also implicitly contained in the set. +/// In particular this means that when intersecting two sets, taking the intersection of some span +/// and one of its subspans returns the subspan, whereas a simple `HashSet` would have returned an +/// empty intersection. +/// It is assumed that two spans don't overlap without one being contained in the other; in other +/// words, that the inclusion structure forms a tree and not a DAG. +/// Intersection is not very efficient. It compares everything pairwise. If needed it could be made +/// faster by sorting the `Span`s and merging cleverly. +#[derive(Debug, Clone, Default)] +pub(crate) struct SpanSet { + /// The minimal set of `Span`s required to represent the whole set. If A and B are `Span`s in + /// the `SpanSet`, and A is a descendant of B, then only B will be in `root_spans`. + /// Invariant: the spans are disjoint. + root_spans: Vec, +} + +impl SpanSet { + /// Creates an empty set. + fn new() -> Self { + Self::default() + } + + /// Tests whether the set is empty. + pub(crate) fn is_empty(&self) -> bool { + self.root_spans.is_empty() + } + + /// Iterate over the disjoint list of spans at the roots of this set. + pub(crate) fn iter<'a>(&'a self) -> impl Iterator + Captures<'a> { + self.root_spans.iter().copied() + } + + /// Tests whether the set contains a given Span. + fn contains(&self, span: Span) -> bool { + self.iter().any(|root_span| root_span.contains(span)) + } + + /// Add a span to the set if we know the span has no intersection in this set. + fn push_nonintersecting(&mut self, new_span: Span) { + self.root_spans.push(new_span); + } + + fn intersection_mut(&mut self, other: &Self) { + if self.is_empty() || other.is_empty() { + *self = Self::new(); + return; + } + // Those that were in `self` but not contained in `other` + let mut leftover = SpanSet::new(); + // We keep the elements in `self` that are also in `other`. + self.root_spans.retain(|span| { + let retain = other.contains(*span); + if !retain { + leftover.root_spans.push(*span); + } + retain + }); + // We keep the elements in `other` that are also in the original `self`. You might think + // this is not needed because `self` already contains the intersection. But those aren't + // just sets of things. If `self = [a]`, `other = [b]` and `a` contains `b`, then `b` + // belongs in the intersection but we didn't catch it in the filtering above. We look at + // `leftover` instead of the full original `self` to avoid duplicates. + for span in other.iter() { + if leftover.contains(span) { + self.root_spans.push(span); + } + } + } +} + #[derive(Clone, Debug)] crate enum Usefulness<'tcx> { - /// Carries, for each column in the matrix, a set of sub-branches that have been found to be - /// unreachable. Used only in the presence of or-patterns, otherwise it stays empty. - Useful(Vec>), + /// Pontentially carries a set of sub-branches that have been found to be unreachable. Used + /// only in the presence of or-patterns, otherwise it stays empty. + Useful(SpanSet), /// Carries a list of witnesses of non-exhaustiveness. UsefulWithWitness(Vec>), NotUseful, @@ -640,14 +710,97 @@ impl<'tcx> Usefulness<'tcx> { fn new_useful(preference: WitnessPreference) -> Self { match preference { ConstructWitness => UsefulWithWitness(vec![Witness(vec![])]), - LeaveOutWitness => Useful(vec![]), + LeaveOutWitness => Useful(Default::default()), } } - fn is_useful(&self) -> bool { - !matches!(*self, NotUseful) + /// When trying several branches and each returns a `Usefulness`, we need to combine the + /// results together. + fn merge(usefulnesses: impl Iterator) -> Self { + // If we have detected some unreachable sub-branches, we only want to keep them when they + // were unreachable in _all_ branches. Eg. in the following, the last `true` is unreachable + // in the second branch of the first or-pattern, but not otherwise. Therefore we don't want + // to lint that it is unreachable. + // ``` + // match (true, true) { + // (true, true) => {} + // (false | true, false | true) => {} + // } + // ``` + // Here however we _do_ want to lint that the last `false` is unreachable. So we don't want + // to intersect the spans that come directly from the or-pattern, since each branch of the + // or-pattern brings a new disjoint pattern. + // ``` + // match None { + // Some(false) => {} + // None | Some(true | false) => {} + // } + // ``` + + // Is `None` when no branch was useful. Will often be `Some(Spanset::new())` because the + // sets are only non-empty in the presence of or-patterns. + let mut unreachables: Option = None; + // Witnesses of usefulness, if any. + let mut witnesses = Vec::new(); + + for u in usefulnesses { + match u { + Useful(spans) if spans.is_empty() => { + // Once we reach the empty set, more intersections won't change the result. + return Useful(SpanSet::new()); + } + Useful(spans) => { + if let Some(unreachables) = &mut unreachables { + if !unreachables.is_empty() { + unreachables.intersection_mut(&spans); + } + if unreachables.is_empty() { + return Useful(SpanSet::new()); + } + } else { + unreachables = Some(spans); + } + } + NotUseful => {} + UsefulWithWitness(wits) => { + witnesses.extend(wits); + } + } + } + + if !witnesses.is_empty() { + UsefulWithWitness(witnesses) + } else if let Some(unreachables) = unreachables { + Useful(unreachables) + } else { + NotUseful + } } + /// After calculating the usefulness for a branch of an or-pattern, call this to make this + /// usefulness mergeable with those from the other branches. + fn unsplit_or_pat(self, this_span: Span, or_pat_spans: &[Span]) -> Self { + match self { + Useful(mut spans) => { + // We register the spans of the other branches of this or-pattern as being + // unreachable from this one. This ensures that intersecting together the sets of + // spans returns what we want. + // Until we optimize `SpanSet` however, intersecting this entails a number of + // comparisons quadratic in the number of branches. + for &span in or_pat_spans { + if span != this_span { + spans.push_nonintersecting(span); + } + } + Useful(spans) + } + x => x, + } + } + + /// After calculating usefulness after a specialization, call this to recontruct a usefulness + /// that makes sense for the matrix pre-specialization. This new usefulness can then be merged + /// with the results of specializing with the other constructors. fn apply_constructor<'p>( self, pcx: PatCtxt<'_, 'p, 'tcx>, @@ -677,23 +830,6 @@ impl<'tcx> Usefulness<'tcx> { }; UsefulWithWitness(new_witnesses) } - Useful(mut unreachables) => { - if !unreachables.is_empty() { - // When we apply a constructor, there are `arity` columns of the matrix that - // corresponded to its arguments. All the unreachables found in these columns - // will, after `apply`, come from the first column. So we take the union of all - // the corresponding sets and put them in the first column. - // Note that `arity` may be 0, in which case we just push a new empty set. - let len = unreachables.len(); - let arity = ctor_wild_subpatterns.len(); - let mut unioned = FxHashSet::default(); - for set in unreachables.drain((len - arity)..) { - unioned.extend(set) - } - unreachables.push(unioned); - } - Useful(unreachables) - } x => x, } } @@ -829,102 +965,37 @@ fn is_useful<'p, 'tcx>( assert!(rows.iter().all(|r| r.len() == v.len())); + // FIXME(Nadrieril): Hack to work around type normalization issues (see #72476). + let ty = matrix.heads().next().map(|r| r.ty).unwrap_or(v.head().ty); + let pcx = PatCtxt { cx, matrix, ty, span: v.head().span, is_top_level }; + + debug!("is_useful_expand_first_col: ty={:#?}, expanding {:#?}", pcx.ty, v.head()); + // If the first pattern is an or-pattern, expand it. - if let Some(vs) = v.expand_or_pat() { + let ret = if let Some(vs) = v.expand_or_pat() { + let subspans: Vec<_> = vs.iter().map(|v| v.head().span).collect(); // We expand the or pattern, trying each of its branches in turn and keeping careful track // of possible unreachable sub-branches. - // - // If two branches have detected some unreachable sub-branches, we need to be careful. If - // they were detected in columns that are not the current one, we want to keep only the - // sub-branches that were unreachable in _all_ branches. Eg. in the following, the last - // `true` is unreachable in the second branch of the first or-pattern, but not otherwise. - // Therefore we don't want to lint that it is unreachable. - // - // ``` - // match (true, true) { - // (true, true) => {} - // (false | true, false | true) => {} - // } - // ``` - // If however the sub-branches come from the current column, they come from the inside of - // the current or-pattern, and we want to keep them all. Eg. in the following, we _do_ want - // to lint that the last `false` is unreachable. - // ``` - // match None { - // Some(false) => {} - // None | Some(true | false) => {} - // } - // ``` - let mut matrix = matrix.clone(); - // We keep track of sub-branches separately depending on whether they come from this column - // or from others. - let mut unreachables_this_column: FxHashSet = FxHashSet::default(); - let mut unreachables_other_columns: Vec> = Vec::default(); - // Whether at least one branch is reachable. - let mut any_is_useful = false; - - for v in vs { - let res = is_useful(cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false); - match res { - Useful(unreachables) => { - if let Some((this_column, other_columns)) = unreachables.split_last() { - // We keep the union of unreachables found in the first column. - unreachables_this_column.extend(this_column); - // We keep the intersection of unreachables found in other columns. - if unreachables_other_columns.is_empty() { - unreachables_other_columns = other_columns.to_vec(); - } else { - unreachables_other_columns = unreachables_other_columns - .into_iter() - .zip(other_columns) - .map(|(x, y)| x.intersection(&y).copied().collect()) - .collect(); - } - } - any_is_useful = true; - } - NotUseful => { - unreachables_this_column.insert(v.head().span); - } - UsefulWithWitness(_) => bug!( - "encountered or-pat in the expansion of `_` during exhaustiveness checking" - ), - } - + let usefulnesses = vs.into_iter().map(|v| { + let v_span = v.head().span; + let usefulness = + is_useful(cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false); // If pattern has a guard don't add it to the matrix. if !is_under_guard { // We push the already-seen patterns into the matrix in order to detect redundant // branches like `Some(_) | Some(0)`. matrix.push(v); } - } - - return if any_is_useful { - let mut unreachables = if unreachables_other_columns.is_empty() { - let n_columns = v.len(); - (0..n_columns - 1).map(|_| FxHashSet::default()).collect() - } else { - unreachables_other_columns - }; - unreachables.push(unreachables_this_column); - Useful(unreachables) - } else { - NotUseful - }; - } - - // FIXME(Nadrieril): Hack to work around type normalization issues (see #72476). - let ty = matrix.heads().next().map(|r| r.ty).unwrap_or(v.head().ty); - let pcx = PatCtxt { cx, matrix, ty, span: v.head().span, is_top_level }; - - debug!("is_useful_expand_first_col: ty={:#?}, expanding {:#?}", pcx.ty, v.head()); - - let ret = v - .head_ctor(cx) - .split(pcx, Some(hir_id)) - .into_iter() - .map(|ctor| { + usefulness.unsplit_or_pat(v_span, &subspans) + }); + Usefulness::merge(usefulnesses) + } else { + // We split the head constructor of `v`. + let ctors = v.head_ctor(cx).split(pcx, Some(hir_id)); + // For each constructor, we compute whether there's a value that starts with it that would + // witness the usefulness of `v`. + let usefulnesses = ctors.into_iter().map(|ctor| { // We cache the result of `Fields::wildcards` because it is used a lot. let ctor_wild_subpatterns = Fields::wildcards(pcx, &ctor); let matrix = pcx.matrix.specialize_constructor(pcx, &ctor, &ctor_wild_subpatterns); @@ -932,9 +1003,9 @@ fn is_useful<'p, 'tcx>( let usefulness = is_useful(pcx.cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false); usefulness.apply_constructor(pcx, &ctor, &ctor_wild_subpatterns) - }) - .find(|result| result.is_useful()) - .unwrap_or(NotUseful); + }); + Usefulness::merge(usefulnesses) + }; debug!("is_useful::returns({:#?}, {:#?}) = {:?}", matrix, v, ret); ret } diff --git a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs index 512f1e283cb46..184ffa85c40e2 100644 --- a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs +++ b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs @@ -64,6 +64,35 @@ fn main() { | 2, ..] => {} _ => {} } + match &[][..] { + [true] => {} + [true | false, ..] => {} + _ => {} + } + match &[][..] { + [false] => {} + [true, ..] => {} + [true //~ ERROR unreachable + | false, ..] => {} + _ => {} + } + match (true, None) { + (true, Some(_)) => {} + (false, Some(true)) => {} + (true | false, None | Some(true //~ ERROR unreachable + | false)) => {} + } + macro_rules! t_or_f { + () => { + (true // FIXME: should be unreachable + | false) + }; + } + match (true, None) { + (true, Some(_)) => {} + (false, Some(true)) => {} + (true | false, None | Some(t_or_f!())) => {} + } match Some(0) { Some(0) => {} Some(0 //~ ERROR unreachable diff --git a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr index e968310d108dd..8b1003b5514a6 100644 --- a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr +++ b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr @@ -95,28 +95,40 @@ LL | [1 | ^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:69:14 + --> $DIR/exhaustiveness-unreachable-pattern.rs:75:10 + | +LL | [true + | ^^^^ + +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:82:36 + | +LL | (true | false, None | Some(true + | ^^^^ + +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:98:14 | LL | Some(0 | ^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:88:19 + --> $DIR/exhaustiveness-unreachable-pattern.rs:117:19 | LL | | false) => {} | ^^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:96:15 + --> $DIR/exhaustiveness-unreachable-pattern.rs:125:15 | LL | | true) => {} | ^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:102:15 + --> $DIR/exhaustiveness-unreachable-pattern.rs:131:15 | LL | | true, | ^^^^ -error: aborting due to 19 previous errors +error: aborting due to 21 previous errors diff --git a/src/test/ui/pattern/usefulness/issue-15129.rs b/src/test/ui/pattern/usefulness/issue-15129.rs index ed134c175eddb..d2b72a86b74cc 100644 --- a/src/test/ui/pattern/usefulness/issue-15129.rs +++ b/src/test/ui/pattern/usefulness/issue-15129.rs @@ -1,17 +1,17 @@ pub enum T { T1(()), - T2(()) + T2(()), } pub enum V { V1(isize), - V2(bool) + V2(bool), } fn main() { match (T::T1(()), V::V2(true)) { - //~^ ERROR non-exhaustive patterns: `(T1(()), V2(_))` not covered + //~^ ERROR non-exhaustive patterns: `(T1(()), V2(_))` and `(T2(()), V1(_))` not covered (T::T1(()), V::V1(i)) => (), - (T::T2(()), V::V2(b)) => () + (T::T2(()), V::V2(b)) => (), } } diff --git a/src/test/ui/pattern/usefulness/issue-15129.stderr b/src/test/ui/pattern/usefulness/issue-15129.stderr index aa4434e72b5c7..79a77240937ac 100644 --- a/src/test/ui/pattern/usefulness/issue-15129.stderr +++ b/src/test/ui/pattern/usefulness/issue-15129.stderr @@ -1,8 +1,8 @@ -error[E0004]: non-exhaustive patterns: `(T1(()), V2(_))` not covered +error[E0004]: non-exhaustive patterns: `(T1(()), V2(_))` and `(T2(()), V1(_))` not covered --> $DIR/issue-15129.rs:12:11 | LL | match (T::T1(()), V::V2(true)) { - | ^^^^^^^^^^^^^^^^^^^^^^^^ pattern `(T1(()), V2(_))` not covered + | ^^^^^^^^^^^^^^^^^^^^^^^^ patterns `(T1(()), V2(_))` and `(T2(()), V1(_))` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms = note: the matched value is of type `(T, V)` diff --git a/src/test/ui/pattern/usefulness/issue-2111.rs b/src/test/ui/pattern/usefulness/issue-2111.rs index 7e5835e8697a3..d27beaeffd637 100644 --- a/src/test/ui/pattern/usefulness/issue-2111.rs +++ b/src/test/ui/pattern/usefulness/issue-2111.rs @@ -1,12 +1,11 @@ fn foo(a: Option, b: Option) { - match (a,b) { - //~^ ERROR: non-exhaustive patterns: `(None, None)` not covered - (Some(a), Some(b)) if a == b => { } - (Some(_), None) | - (None, Some(_)) => { } - } + match (a, b) { + //~^ ERROR: non-exhaustive patterns: `(None, None)` and `(Some(_), Some(_))` not covered + (Some(a), Some(b)) if a == b => {} + (Some(_), None) | (None, Some(_)) => {} + } } fn main() { - foo(None, None); + foo(None, None); } diff --git a/src/test/ui/pattern/usefulness/issue-2111.stderr b/src/test/ui/pattern/usefulness/issue-2111.stderr index a39a479e078d9..60d9b8514b7fb 100644 --- a/src/test/ui/pattern/usefulness/issue-2111.stderr +++ b/src/test/ui/pattern/usefulness/issue-2111.stderr @@ -1,8 +1,8 @@ -error[E0004]: non-exhaustive patterns: `(None, None)` not covered - --> $DIR/issue-2111.rs:2:9 +error[E0004]: non-exhaustive patterns: `(None, None)` and `(Some(_), Some(_))` not covered + --> $DIR/issue-2111.rs:2:11 | -LL | match (a,b) { - | ^^^^^ pattern `(None, None)` not covered +LL | match (a, b) { + | ^^^^^^ patterns `(None, None)` and `(Some(_), Some(_))` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms = note: the matched value is of type `(Option, Option)` diff --git a/src/test/ui/pattern/usefulness/issue-56379.rs b/src/test/ui/pattern/usefulness/issue-56379.rs new file mode 100644 index 0000000000000..9bccccca9c2b6 --- /dev/null +++ b/src/test/ui/pattern/usefulness/issue-56379.rs @@ -0,0 +1,14 @@ +enum Foo { + A(bool), + B(bool), + C(bool), +} + +fn main() { + match Foo::A(true) { + //~^ ERROR non-exhaustive patterns: `A(false)`, `B(false)` and `C(false)` not covered + Foo::A(true) => {} + Foo::B(true) => {} + Foo::C(true) => {} + } +} diff --git a/src/test/ui/pattern/usefulness/issue-56379.stderr b/src/test/ui/pattern/usefulness/issue-56379.stderr new file mode 100644 index 0000000000000..6a231b868c8c4 --- /dev/null +++ b/src/test/ui/pattern/usefulness/issue-56379.stderr @@ -0,0 +1,22 @@ +error[E0004]: non-exhaustive patterns: `A(false)`, `B(false)` and `C(false)` not covered + --> $DIR/issue-56379.rs:8:11 + | +LL | / enum Foo { +LL | | A(bool), + | | - not covered +LL | | B(bool), + | | - not covered +LL | | C(bool), + | | - not covered +LL | | } + | |_- `Foo` defined here +... +LL | match Foo::A(true) { + | ^^^^^^^^^^^^ patterns `A(false)`, `B(false)` and `C(false)` not covered + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + = note: the matched value is of type `Foo` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0004`. diff --git a/src/test/ui/pattern/usefulness/non-exhaustive-match.rs b/src/test/ui/pattern/usefulness/non-exhaustive-match.rs index a28cfb579f4f1..4ff12aa2ff5e2 100644 --- a/src/test/ui/pattern/usefulness/non-exhaustive-match.rs +++ b/src/test/ui/pattern/usefulness/non-exhaustive-match.rs @@ -15,7 +15,7 @@ fn main() { // and `(_, _, 5_i32..=i32::MAX)` not covered (_, _, 4) => {} } - match (T::A, T::A) { //~ ERROR non-exhaustive patterns: `(A, A)` not covered + match (T::A, T::A) { //~ ERROR non-exhaustive patterns: `(A, A)` and `(B, B)` not covered (T::A, T::B) => {} (T::B, T::A) => {} } diff --git a/src/test/ui/pattern/usefulness/non-exhaustive-match.stderr b/src/test/ui/pattern/usefulness/non-exhaustive-match.stderr index 12412743b83fb..c953cd314406e 100644 --- a/src/test/ui/pattern/usefulness/non-exhaustive-match.stderr +++ b/src/test/ui/pattern/usefulness/non-exhaustive-match.stderr @@ -45,11 +45,11 @@ LL | match (2, 3, 4) { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms = note: the matched value is of type `(i32, i32, i32)` -error[E0004]: non-exhaustive patterns: `(A, A)` not covered +error[E0004]: non-exhaustive patterns: `(A, A)` and `(B, B)` not covered --> $DIR/non-exhaustive-match.rs:18:11 | LL | match (T::A, T::A) { - | ^^^^^^^^^^^^ pattern `(A, A)` not covered + | ^^^^^^^^^^^^ patterns `(A, A)` and `(B, B)` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms = note: the matched value is of type `(T, T)`