Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve and fix diagnostics of exhaustiveness checking #80104

Merged
merged 7 commits into from
Dec 19, 2020
Merged

Improve and fix diagnostics of exhaustiveness checking #80104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2309783
Add tests
Nadrieril Dec 17, 2020
7009d20
Factor out or-pattern usefulness merging
Nadrieril Dec 16, 2020
170fae2
Log the output of `is_useful` in the or-pattern case too
Nadrieril Dec 17, 2020
d7a6365
Rewrite usefulness merging using `SpanSet`
Nadrieril Dec 17, 2020
2d71a0b
Keep all witnesses of non-exhaustiveness
Nadrieril Dec 17, 2020
6319d73
Merge unreachable subpatterns correctly
Nadrieril Dec 17, 2020
cefcadb
Unify the two kinds of usefulness merging
Nadrieril Dec 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/thir/pattern/check_match.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
297 changes: 184 additions & 113 deletions compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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>,
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -829,112 +965,47 @@ 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<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"
),
}

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);
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)
};
debug!("is_useful::returns({:#?}, {:#?}) = {:?}", matrix, v, ret);
ret
}
Expand Down
Loading