Auto merge of #80104 - Nadrieril:usefulness-merging, r=varkor

Improve and fix diagnostics of exhaustiveness checking

Primarily, this fixes https://github.com/rust-lang/rust/issues/56379. This also fixes incorrect interactions between or-patterns and slice patterns that I discovered while working on #56379. Those two examples show the incorrect diagnostics:

```rust
match &[][..] {
    [true] => {}
    [true // detected as unreachable but that's not true
        | false, ..] => {}
    _ => {}
}
match (true, None) {
    (true, Some(_)) => {}
    (false, Some(true)) => {}
    (true | false, None | Some(true // should be detected as unreachable
                               | false)) => {}
}
```

I did not measure any perf impact. However, I suspect that [`616ba9f`](616ba9f9f7) should have a negative impact on large or-patterns. I'll see what the perf run says; I have optimization ideas up my sleeve if needed.

EDIT: I initially had a noticeable perf impact that I thought unavoidable. I then proceeded to avoid it x)

r? `@varkor`
`@rustbot` label +A-exhaustiveness-checking
This commit is contained in:
bors 2020-12-19 19:14:04 +00:00
commit 1f5bc176b0
12 changed files with 290 additions and 143 deletions

View File

@ -433,7 +433,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 {

View File

@ -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<PatStack<'p, 'tcx>> 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<Span>,
}
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<Item = Span> + 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<FxHashSet<Span>>),
/// 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<Witness<'tcx>>),
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<Item = Self>) -> 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<SpanSet> = 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()));
// If the first pattern is an or-pattern, expand it.
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<Span> = FxHashSet::default();
let mut unreachables_other_columns: Vec<FxHashSet<Span>> = 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"
),
}
// 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| {
// 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 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);
}
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
}

View File

@ -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

View File

@ -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

View File

@ -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)) => (),
}
}

View File

@ -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)`

View File

@ -1,12 +1,11 @@
fn foo(a: Option<usize>, b: Option<usize>) {
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);
}

View File

@ -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<usize>, Option<usize>)`

View File

@ -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) => {}
}
}

View File

@ -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`.

View File

@ -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) => {}
}

View File

@ -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)`