From 2309783a0b65163041b03dce04d7df85dcabc2cd Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 17 Dec 2020 00:42:49 +0000 Subject: [PATCH 1/7] Add tests --- .../exhaustiveness-unreachable-pattern.rs | 31 +++++++++++++++++++ .../exhaustiveness-unreachable-pattern.stderr | 22 ++++++++++--- src/test/ui/pattern/usefulness/issue-15129.rs | 8 ++--- src/test/ui/pattern/usefulness/issue-2111.rs | 13 ++++---- .../ui/pattern/usefulness/issue-2111.stderr | 6 ++-- src/test/ui/pattern/usefulness/issue-56379.rs | 14 +++++++++ .../ui/pattern/usefulness/issue-56379.stderr | 20 ++++++++++++ 7 files changed, 95 insertions(+), 19 deletions(-) create mode 100644 src/test/ui/pattern/usefulness/issue-56379.rs create mode 100644 src/test/ui/pattern/usefulness/issue-56379.stderr diff --git a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs index 512f1e283cb46..178a4aa6681a0 100644 --- a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs +++ b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs @@ -64,6 +64,37 @@ fn main() { | 2, ..] => {} _ => {} } + // FIXME: incorrect + match &[][..] { + [true] => {} + [true //~ ERROR unreachable + | false, ..] => {} + _ => {} + } + match &[][..] { + [false] => {} + [true, ..] => {} + [true //~ ERROR unreachable + | false, ..] => {} + _ => {} + } + match (true, None) { + (true, Some(_)) => {} + (false, Some(true)) => {} + (true | false, None | Some(true // FIXME: should be 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..38e2369ae7dba 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:70:10 + | +LL | [true + | ^^^^ + +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:77:10 + | +LL | [true + | ^^^^ + +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:100:14 | LL | Some(0 | ^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:88:19 + --> $DIR/exhaustiveness-unreachable-pattern.rs:119:19 | LL | | false) => {} | ^^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:96:15 + --> $DIR/exhaustiveness-unreachable-pattern.rs:127:15 | LL | | true) => {} | ^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:102:15 + --> $DIR/exhaustiveness-unreachable-pattern.rs:133: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..bcfc32be9a483 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(_))` 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-2111.rs b/src/test/ui/pattern/usefulness/issue-2111.rs index 7e5835e8697a3..0847045cdaa91 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)` 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..f0609ccebc162 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 + --> $DIR/issue-2111.rs:2:11 | -LL | match (a,b) { - | ^^^^^ pattern `(None, None)` not covered +LL | match (a, b) { + | ^^^^^^ pattern `(None, None)` 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..5454e80cdb4f3 --- /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)` 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..661e0dbb43923 --- /dev/null +++ b/src/test/ui/pattern/usefulness/issue-56379.stderr @@ -0,0 +1,20 @@ +error[E0004]: non-exhaustive patterns: `A(false)` not covered + --> $DIR/issue-56379.rs:8:11 + | +LL | / enum Foo { +LL | | A(bool), + | | - not covered +LL | | B(bool), +LL | | C(bool), +LL | | } + | |_- `Foo` defined here +... +LL | match Foo::A(true) { + | ^^^^^^^^^^^^ pattern `A(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`. From 7009d202904e9af500ef13285b4c50cf63c2e75b Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 16 Dec 2020 06:24:31 +0000 Subject: [PATCH 2/7] Factor out or-pattern usefulness merging --- .../src/thir/pattern/usefulness.rs | 153 +++++++++--------- 1 file changed, 81 insertions(+), 72 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs index f3e1507b37ae1..ae92898a0f413 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs @@ -648,6 +648,81 @@ impl<'tcx> Usefulness<'tcx> { !matches!(*self, NotUseful) } + /// When trying several branches and each returns a `Usefulness`, we need to combine the + /// results together. + fn merge(usefulnesses: impl Iterator, column_count: usize) -> Self { + // 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) => {} + // } + // ``` + + // 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 (u, span) in usefulnesses { + match u { + 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(span); + } + UsefulWithWitness(_) => { + bug!( + "encountered or-pat in the expansion of `_` during exhaustiveness checking" + ) + } + } + } + + if any_is_useful { + let mut unreachables = if unreachables_other_columns.is_empty() { + (0..column_count - 1).map(|_| FxHashSet::default()).collect() + } else { + unreachables_other_columns + }; + unreachables.push(unreachables_this_column); + Useful(unreachables) + } else { + NotUseful + } + } + fn apply_constructor<'p>( self, pcx: PatCtxt<'_, 'p, 'tcx>, @@ -833,85 +908,19 @@ fn is_useful<'p, 'tcx>( if let Some(vs) = v.expand_or_pat() { // 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 span = v.head().span; + let u = 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 - }; + (u, span) + }); + return Usefulness::merge(usefulnesses, v.len()); } // FIXME(Nadrieril): Hack to work around type normalization issues (see #72476). From 170fae2c187936498ffdebbbd7e3cad47af31cc9 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 17 Dec 2020 00:47:31 +0000 Subject: [PATCH 3/7] Log the output of `is_useful` in the or-pattern case too --- .../src/thir/pattern/usefulness.rs | 56 ++++++++++--------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs index ae92898a0f413..9646f7ffcf7fb 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs @@ -904,8 +904,14 @@ 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() { // We expand the or pattern, trying each of its branches in turn and keeping careful track // of possible unreachable sub-branches. let mut matrix = matrix.clone(); @@ -920,30 +926,30 @@ fn is_useful<'p, 'tcx>( } (u, span) }); - return Usefulness::merge(usefulnesses, 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()); - - let ret = v - .head_ctor(cx) - .split(pcx, Some(hir_id)) - .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); - let v = v.pop_head_constructor(&ctor_wild_subpatterns); - 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, v.len()) + } else { + v.head_ctor(cx) + .split(pcx, Some(hir_id)) + .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); + let v = v.pop_head_constructor(&ctor_wild_subpatterns); + 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) + }; debug!("is_useful::returns({:#?}, {:#?}) = {:?}", matrix, v, ret); ret } From d7a6365b77e171337d4f4220ebdc618965524ecd Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 17 Dec 2020 01:07:49 +0000 Subject: [PATCH 4/7] Rewrite usefulness merging using `SpanSet` `SpanSet` is heavily inspired from `DefIdForest`. --- .../src/thir/pattern/check_match.rs | 2 +- .../src/thir/pattern/usefulness.rs | 177 +++++++++++------- .../exhaustiveness-unreachable-pattern.rs | 2 +- .../exhaustiveness-unreachable-pattern.stderr | 8 +- 4 files changed, 122 insertions(+), 67 deletions(-) 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 9646f7ffcf7fb..5af155bd746a3 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,81 @@ 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. +/// Operations on this do not need to be fast since it's only nonempty in the diagnostic path. +#[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,7 +709,7 @@ impl<'tcx> Usefulness<'tcx> { fn new_useful(preference: WitnessPreference) -> Self { match preference { ConstructWitness => UsefulWithWitness(vec![Witness(vec![])]), - LeaveOutWitness => Useful(vec![]), + LeaveOutWitness => Useful(Default::default()), } } @@ -650,22 +719,20 @@ impl<'tcx> Usefulness<'tcx> { /// When trying several branches and each returns a `Usefulness`, we need to combine the /// results together. - fn merge(usefulnesses: impl Iterator, column_count: usize) -> Self { - // 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. - // + 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) => {} // } // ``` - // 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. + // 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) => {} @@ -673,35 +740,34 @@ impl<'tcx> Usefulness<'tcx> { // } // ``` - // 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; + // Is `None` when no branch was useful. Will often be `Some(Spanset::new())` because the + // sets are only non-empty in the diagnostic path. + let mut unreachables: Option = None; + // In case of or-patterns we don't want to intersect subpatterns that come from the first + // column. Invariant: contains a list of disjoint spans. + let mut unreachables_this_column = Vec::new(); - for (u, span) in usefulnesses { + for (u, branch_span) in usefulnesses { match u { - 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(); + Useful(spans) if spans.is_empty() => { + // Hot path: `spans` is only non-empty in the diagnostic path. + unreachables = Some(SpanSet::new()); + } + Useful(spans) => { + for span in spans.iter() { + if branch_span.contains(span) { + unreachables_this_column.push(span) } } - any_is_useful = true; - } - NotUseful => { - unreachables_this_column.insert(span); + if let Some(set) = &mut unreachables { + if !set.is_empty() { + set.intersection_mut(&spans); + } + } else { + unreachables = Some(spans); + } } + NotUseful => unreachables_this_column.push(branch_span), UsefulWithWitness(_) => { bug!( "encountered or-pat in the expansion of `_` during exhaustiveness checking" @@ -710,13 +776,13 @@ impl<'tcx> Usefulness<'tcx> { } } - if any_is_useful { - let mut unreachables = if unreachables_other_columns.is_empty() { - (0..column_count - 1).map(|_| FxHashSet::default()).collect() - } else { - unreachables_other_columns - }; - unreachables.push(unreachables_this_column); + if let Some(mut unreachables) = unreachables { + for span in unreachables_this_column { + // `unreachables` contained no spans from the first column, and + // `unreachables_this_column` contains only disjoint spans. Therefore it is valid + // to call `push_nonintersecting`. + unreachables.push_nonintersecting(span); + } Useful(unreachables) } else { NotUseful @@ -752,23 +818,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, } } @@ -926,7 +975,7 @@ fn is_useful<'p, 'tcx>( } (u, span) }); - Usefulness::merge(usefulnesses, v.len()) + Usefulness::merge(usefulnesses) } else { v.head_ctor(cx) .split(pcx, Some(hir_id)) diff --git a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs index 178a4aa6681a0..3e242a797a201 100644 --- a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs +++ b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs @@ -81,7 +81,7 @@ fn main() { match (true, None) { (true, Some(_)) => {} (false, Some(true)) => {} - (true | false, None | Some(true // FIXME: should be unreachable + (true | false, None | Some(true //~ ERROR unreachable | false)) => {} } macro_rules! t_or_f { diff --git a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr index 38e2369ae7dba..ac0b34c2bd541 100644 --- a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr +++ b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr @@ -106,6 +106,12 @@ error: unreachable pattern LL | [true | ^^^^ +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:84:36 + | +LL | (true | false, None | Some(true + | ^^^^ + error: unreachable pattern --> $DIR/exhaustiveness-unreachable-pattern.rs:100:14 | @@ -130,5 +136,5 @@ error: unreachable pattern LL | | true, | ^^^^ -error: aborting due to 21 previous errors +error: aborting due to 22 previous errors From 2d71a0b9b9871966dd1f63b4113eeecc2d6e2f6f Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 17 Dec 2020 01:18:01 +0000 Subject: [PATCH 5/7] Keep all witnesses of non-exhaustiveness --- .../src/thir/pattern/usefulness.rs | 64 +++++++++++-------- src/test/ui/pattern/usefulness/issue-15129.rs | 2 +- .../ui/pattern/usefulness/issue-15129.stderr | 4 +- src/test/ui/pattern/usefulness/issue-2111.rs | 2 +- .../ui/pattern/usefulness/issue-2111.stderr | 4 +- src/test/ui/pattern/usefulness/issue-56379.rs | 2 +- .../ui/pattern/usefulness/issue-56379.stderr | 6 +- .../usefulness/non-exhaustive-match.rs | 2 +- .../usefulness/non-exhaustive-match.stderr | 4 +- 9 files changed, 51 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs index 5af155bd746a3..02b5e0eb3b22b 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs @@ -713,13 +713,9 @@ impl<'tcx> Usefulness<'tcx> { } } - 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 { + fn merge_or_patterns(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 @@ -789,6 +785,27 @@ impl<'tcx> Usefulness<'tcx> { } } + /// When trying several branches and each returns a `Usefulness`, we need to combine the + /// results together. + fn merge_split_constructors(usefulnesses: impl Iterator) -> Self { + // Witnesses of usefulness, if any. + let mut witnesses = Vec::new(); + + for u in usefulnesses { + match u { + Useful(..) => { + return u; + } + NotUseful => {} + UsefulWithWitness(wits) => { + witnesses.extend(wits); + } + } + } + + if !witnesses.is_empty() { UsefulWithWitness(witnesses) } else { NotUseful } + } + fn apply_constructor<'p>( self, pcx: PatCtxt<'_, 'p, 'tcx>, @@ -975,29 +992,22 @@ fn is_useful<'p, 'tcx>( } (u, span) }); - Usefulness::merge(usefulnesses) + Usefulness::merge_or_patterns(usefulnesses) } else { - v.head_ctor(cx) - .split(pcx, Some(hir_id)) - .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); - let v = v.pop_head_constructor(&ctor_wild_subpatterns); - 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) + // 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); + let v = v.pop_head_constructor(&ctor_wild_subpatterns); + let usefulness = + is_useful(pcx.cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false); + usefulness.apply_constructor(pcx, &ctor, &ctor_wild_subpatterns) + }); + Usefulness::merge_split_constructors(usefulnesses) }; debug!("is_useful::returns({:#?}, {:#?}) = {:?}", matrix, v, ret); ret diff --git a/src/test/ui/pattern/usefulness/issue-15129.rs b/src/test/ui/pattern/usefulness/issue-15129.rs index bcfc32be9a483..d2b72a86b74cc 100644 --- a/src/test/ui/pattern/usefulness/issue-15129.rs +++ b/src/test/ui/pattern/usefulness/issue-15129.rs @@ -10,7 +10,7 @@ pub enum V { 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)) => (), } 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 0847045cdaa91..d27beaeffd637 100644 --- a/src/test/ui/pattern/usefulness/issue-2111.rs +++ b/src/test/ui/pattern/usefulness/issue-2111.rs @@ -1,6 +1,6 @@ fn foo(a: Option, b: Option) { match (a, b) { - //~^ ERROR: non-exhaustive patterns: `(None, None)` not covered + //~^ ERROR: non-exhaustive patterns: `(None, None)` and `(Some(_), Some(_))` not covered (Some(a), Some(b)) if a == b => {} (Some(_), None) | (None, Some(_)) => {} } diff --git a/src/test/ui/pattern/usefulness/issue-2111.stderr b/src/test/ui/pattern/usefulness/issue-2111.stderr index f0609ccebc162..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 +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 + | ^^^^^^ 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 index 5454e80cdb4f3..9bccccca9c2b6 100644 --- a/src/test/ui/pattern/usefulness/issue-56379.rs +++ b/src/test/ui/pattern/usefulness/issue-56379.rs @@ -6,7 +6,7 @@ enum Foo { fn main() { match Foo::A(true) { - //~^ ERROR non-exhaustive patterns: `A(false)` not covered + //~^ 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 index 661e0dbb43923..6a231b868c8c4 100644 --- a/src/test/ui/pattern/usefulness/issue-56379.stderr +++ b/src/test/ui/pattern/usefulness/issue-56379.stderr @@ -1,16 +1,18 @@ -error[E0004]: non-exhaustive patterns: `A(false)` not covered +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) { - | ^^^^^^^^^^^^ pattern `A(false)` not covered + | ^^^^^^^^^^^^ 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` 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)` From 6319d737e07d732aa044864933b69a39fdbeec0a Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 17 Dec 2020 01:25:53 +0000 Subject: [PATCH 6/7] Merge unreachable subpatterns correctly --- .../src/thir/pattern/usefulness.rs | 31 +++++++++++++++++-- .../exhaustiveness-unreachable-pattern.rs | 4 +-- .../exhaustiveness-unreachable-pattern.stderr | 20 +++++------- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs index 02b5e0eb3b22b..c07d4c9b8f74b 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs @@ -788,13 +788,32 @@ impl<'tcx> Usefulness<'tcx> { /// When trying several branches and each returns a `Usefulness`, we need to combine the /// results together. fn merge_split_constructors(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. So we take a big intersection. + + // Is `None` when no branch was useful. Will often be `Some(Spanset::new())` because the + // sets are only non-empty in the diagnostic path. + let mut unreachables: Option = None; // Witnesses of usefulness, if any. let mut witnesses = Vec::new(); for u in usefulnesses { match u { - Useful(..) => { - return 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) => { @@ -803,7 +822,13 @@ impl<'tcx> Usefulness<'tcx> { } } - if !witnesses.is_empty() { UsefulWithWitness(witnesses) } else { NotUseful } + if !witnesses.is_empty() { + UsefulWithWitness(witnesses) + } else if let Some(unreachables) = unreachables { + Useful(unreachables) + } else { + NotUseful + } } fn apply_constructor<'p>( diff --git a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs index 3e242a797a201..184ffa85c40e2 100644 --- a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs +++ b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs @@ -64,11 +64,9 @@ fn main() { | 2, ..] => {} _ => {} } - // FIXME: incorrect match &[][..] { [true] => {} - [true //~ ERROR unreachable - | false, ..] => {} + [true | false, ..] => {} _ => {} } match &[][..] { diff --git a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr index ac0b34c2bd541..8b1003b5514a6 100644 --- a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr +++ b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr @@ -95,46 +95,40 @@ LL | [1 | ^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:70:10 + --> $DIR/exhaustiveness-unreachable-pattern.rs:75:10 | LL | [true | ^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:77:10 - | -LL | [true - | ^^^^ - -error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:84:36 + --> $DIR/exhaustiveness-unreachable-pattern.rs:82:36 | LL | (true | false, None | Some(true | ^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:100:14 + --> $DIR/exhaustiveness-unreachable-pattern.rs:98:14 | LL | Some(0 | ^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:119:19 + --> $DIR/exhaustiveness-unreachable-pattern.rs:117:19 | LL | | false) => {} | ^^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:127:15 + --> $DIR/exhaustiveness-unreachable-pattern.rs:125:15 | LL | | true) => {} | ^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:133:15 + --> $DIR/exhaustiveness-unreachable-pattern.rs:131:15 | LL | | true, | ^^^^ -error: aborting due to 22 previous errors +error: aborting due to 21 previous errors From cefcadbe9278cf30175e93309cdd5f1da6714869 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 17 Dec 2020 01:56:22 +0000 Subject: [PATCH 7/7] Unify the two kinds of usefulness merging This is elegant but a bit of a perf gamble. That said, or-patterns rarely have many branches and it's easy to optimize or revert if we ever need to. In the meantime simpler code is worth it. --- .../src/thir/pattern/usefulness.rs | 98 +++++++------------ 1 file changed, 35 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs index c07d4c9b8f74b..0ecc034ac0ba9 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs @@ -632,7 +632,8 @@ impl<'p, 'tcx> FromIterator> for Matrix<'p, 'tcx> { /// 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. -/// Operations on this do not need to be fast since it's only nonempty in the diagnostic path. +/// 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 @@ -715,7 +716,7 @@ impl<'tcx> Usefulness<'tcx> { /// When trying several branches and each returns a `Usefulness`, we need to combine the /// results together. - fn merge_or_patterns(usefulnesses: impl Iterator) -> Self { + 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 @@ -737,62 +738,7 @@ impl<'tcx> Usefulness<'tcx> { // ``` // Is `None` when no branch was useful. Will often be `Some(Spanset::new())` because the - // sets are only non-empty in the diagnostic path. - let mut unreachables: Option = None; - // In case of or-patterns we don't want to intersect subpatterns that come from the first - // column. Invariant: contains a list of disjoint spans. - let mut unreachables_this_column = Vec::new(); - - for (u, branch_span) in usefulnesses { - match u { - Useful(spans) if spans.is_empty() => { - // Hot path: `spans` is only non-empty in the diagnostic path. - unreachables = Some(SpanSet::new()); - } - Useful(spans) => { - for span in spans.iter() { - if branch_span.contains(span) { - unreachables_this_column.push(span) - } - } - if let Some(set) = &mut unreachables { - if !set.is_empty() { - set.intersection_mut(&spans); - } - } else { - unreachables = Some(spans); - } - } - NotUseful => unreachables_this_column.push(branch_span), - UsefulWithWitness(_) => { - bug!( - "encountered or-pat in the expansion of `_` during exhaustiveness checking" - ) - } - } - } - - if let Some(mut unreachables) = unreachables { - for span in unreachables_this_column { - // `unreachables` contained no spans from the first column, and - // `unreachables_this_column` contains only disjoint spans. Therefore it is valid - // to call `push_nonintersecting`. - unreachables.push_nonintersecting(span); - } - Useful(unreachables) - } else { - NotUseful - } - } - - /// When trying several branches and each returns a `Usefulness`, we need to combine the - /// results together. - fn merge_split_constructors(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. So we take a big intersection. - - // Is `None` when no branch was useful. Will often be `Some(Spanset::new())` because the - // sets are only non-empty in the diagnostic path. + // 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(); @@ -831,6 +777,30 @@ impl<'tcx> Usefulness<'tcx> { } } + /// 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>, @@ -1003,21 +973,23 @@ fn is_useful<'p, 'tcx>( // If the first pattern is an or-pattern, expand it. 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. let mut matrix = matrix.clone(); let usefulnesses = vs.into_iter().map(|v| { - let span = v.head().span; - let u = is_useful(cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false); + 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); } - (u, span) + usefulness.unsplit_or_pat(v_span, &subspans) }); - Usefulness::merge_or_patterns(usefulnesses) + Usefulness::merge(usefulnesses) } else { // We split the head constructor of `v`. let ctors = v.head_ctor(cx).split(pcx, Some(hir_id)); @@ -1032,7 +1004,7 @@ fn is_useful<'p, 'tcx>( is_useful(pcx.cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false); usefulness.apply_constructor(pcx, &ctor, &ctor_wild_subpatterns) }); - Usefulness::merge_split_constructors(usefulnesses) + Usefulness::merge(usefulnesses) }; debug!("is_useful::returns({:#?}, {:#?}) = {:?}", matrix, v, ret); ret