Skip to content

Commit

Permalink
Make SubPatSet clearer by flipping its meaning
Browse files Browse the repository at this point in the history
  • Loading branch information
Nadrieril committed Feb 1, 2021
1 parent 37e7dd2 commit ae6fcab
Showing 1 changed file with 119 additions and 94 deletions.
213 changes: 119 additions & 94 deletions compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,34 +619,45 @@ impl<'p, 'tcx> FromIterator<PatStack<'p, 'tcx>> for Matrix<'p, 'tcx> {
}
}

/// Given a pattern or a pattern-stack, this struct captures a set of its subpattern branches. We
/// use that to track unreachable sub-patterns arising from or-patterns. In the absence of
/// or-patterns this will always be either `Empty` or `Full`.
/// We support a limited set of operations, so not all possible sets of subpatterns can be
/// represented. That's ok, we only want the ones that make sense to capture unreachable
/// subpatterns.
/// What we're trying to do is illustrated by this:
/// Given a pattern or a pattern-stack, this struct captures a set of its subpatterns. We use that
/// to track reachable sub-patterns arising from or-patterns. In the absence of or-patterns this
/// will always be either `Empty` (the whole pattern is unreachable) or `Full` (the whole pattern
/// is reachable). When there are or-patterns, some subpatterns may be reachable while others
/// aren't. In this case the whole pattern still counts as reachable, but we will lint the
/// unreachable subpatterns.
///
/// This supports a limited set of operations, so not all possible sets of subpatterns can be
/// represented. That's ok, we only want the ones that make sense for our usage.
///
/// What we're doing is illustrated by this:
/// ```
/// match (true, true) {
/// (true, true) => {}
/// (true | false, true | false) => {}
/// match (true, 0) {
/// (true, 0) => {}
/// (_, 1) => {}
/// (true | false, 0 | 1) => {}
/// }
/// ```
/// When we try the alternatives of the first or-pattern, the last `true` is unreachable in the
/// first alternative but no the other. So we don't want to report it as unreachable. Therefore we
/// intersect sets of unreachable patterns coming from different alternatives in order to figure
/// out which subpatterns are overall unreachable.
/// When we try the alternatives of the `true | false` or-pattern, the last `0` is reachable in the
/// `false` alternative but not the `true`. So overall it is reachable. By contrast, the last `1`
/// is not reachable in either alternative, so we want to signal this to the user.
/// Therefore we take the union of sets of reachable patterns coming from different alternatives in
/// order to figure out which subpatterns are overall reachable.
///
/// Invariant: we try to construct the smallest representation we can. In particular if
/// `self.is_empty()` we ensure that `self` is `Empty`, and same with `Full`. This is not important
/// for correctness currently.
#[derive(Debug, Clone)]
enum SubPatSet<'p, 'tcx> {
/// The empty set. This means the pattern is unreachable.
Empty,
/// The set containing the full pattern.
Full,
/// The empty set.
Empty,
/// If the pattern is a pattern with a constructor or a pattern-stack, we store a set for each
/// of its subpatterns. Missing entries in the map are implicitly empty.
/// of its subpatterns. Missing entries in the map are implicitly full, because that's the
/// common case.
Seq { subpats: FxHashMap<usize, SubPatSet<'p, 'tcx>> },
/// If the pattern is an or-pattern, we store a set for each of its alternatives. Missing
/// entries in the map are implicitly full. Note: we always flatten nested or-patterns.
/// entries in the map are implicitly empty. Note: we always flatten nested or-patterns.
Alt {
subpats: FxHashMap<usize, SubPatSet<'p, 'tcx>>,
/// Counts the total number of alternatives in the pattern
Expand All @@ -657,88 +668,91 @@ enum SubPatSet<'p, 'tcx> {
}

impl<'p, 'tcx> SubPatSet<'p, 'tcx> {
fn empty() -> Self {
SubPatSet::Empty
}
fn full() -> Self {
SubPatSet::Full
}
fn empty() -> Self {
SubPatSet::Empty
}

fn is_full(&self) -> bool {
fn is_empty(&self) -> bool {
match self {
SubPatSet::Full => true,
SubPatSet::Empty => false,
SubPatSet::Empty => true,
SubPatSet::Full => false,
// If any subpattern in a sequence is unreachable, the whole pattern is unreachable.
SubPatSet::Seq { subpats } => subpats.values().any(|set| set.is_full()),
SubPatSet::Alt { subpats, .. } => subpats.values().all(|set| set.is_full()),
SubPatSet::Seq { subpats } => subpats.values().any(|set| set.is_empty()),
// An or-pattern is reachable if any of its alternatives is.
SubPatSet::Alt { subpats, .. } => subpats.values().all(|set| set.is_empty()),
}
}

fn is_empty(&self) -> bool {
fn is_full(&self) -> bool {
match self {
SubPatSet::Full => false,
SubPatSet::Empty => true,
SubPatSet::Seq { subpats } => subpats.values().all(|sub_set| sub_set.is_empty()),
SubPatSet::Empty => false,
SubPatSet::Full => true,
// The whole pattern is reachable only when all its alternatives are.
SubPatSet::Seq { subpats } => subpats.values().all(|sub_set| sub_set.is_full()),
// The whole or-pattern is reachable only when all its alternatives are.
SubPatSet::Alt { subpats, alt_count, .. } => {
subpats.len() == *alt_count && subpats.values().all(|set| set.is_empty())
subpats.len() == *alt_count && subpats.values().all(|set| set.is_full())
}
}
}

/// Intersect `self` with `other`, mutating `self`.
fn intersect(&mut self, other: Self) {
/// Union `self` with `other`, mutating `self`.
fn union(&mut self, other: Self) {
use SubPatSet::*;
// Intersecting with empty stays empty; intersecting with full changes nothing.
if self.is_empty() || other.is_full() {
// Union with full stays full; union with empty changes nothing.
if self.is_full() || other.is_empty() {
return;
} else if self.is_full() {
} else if self.is_empty() {
*self = other;
return;
} else if other.is_empty() {
*self = Empty;
} else if other.is_full() {
*self = Full;
return;
}

match (&mut *self, other) {
(Seq { subpats: s_set }, Seq { subpats: mut o_set }) => {
s_set.retain(|i, s_sub_set| {
// Missing entries count as empty.
let o_sub_set = o_set.remove(&i).unwrap_or(Empty);
s_sub_set.intersect(o_sub_set);
// We drop empty entries.
!s_sub_set.is_empty()
});
// Everything left in `o_set` is missing from `s_set`, i.e. counts as empty. Since
// intersecting with empty returns empty, we can drop those entries.
}
(Alt { subpats: s_set, .. }, Alt { subpats: mut o_set, .. }) => {
s_set.retain(|i, s_sub_set| {
// Missing entries count as full.
let o_sub_set = o_set.remove(&i).unwrap_or(Full);
s_sub_set.intersect(o_sub_set);
s_sub_set.union(o_sub_set);
// We drop full entries.
!s_sub_set.is_full()
});
// Everything left in `o_set` is missing from `s_set`, i.e. counts as full. Since
// intersecting with full changes nothing, we can take those entries as is.
// unioning with full returns full, we can drop those entries.
}
(Alt { subpats: s_set, .. }, Alt { subpats: mut o_set, .. }) => {
s_set.retain(|i, s_sub_set| {
// Missing entries count as empty.
let o_sub_set = o_set.remove(&i).unwrap_or(Empty);
s_sub_set.union(o_sub_set);
// We drop empty entries.
!s_sub_set.is_empty()
});
// Everything left in `o_set` is missing from `s_set`, i.e. counts as empty. Since
// unioning with empty changes nothing, we can take those entries as is.
s_set.extend(o_set);
}
_ => bug!(),
}

if self.is_empty() {
*self = Empty;
if self.is_full() {
*self = Full;
}
}

/// Returns a list of the spans of the unreachable subpatterns. If `self` is full we return
/// `None`.
fn to_spans(&self) -> Option<Vec<Span>> {
/// Panics if `set.is_full()`.
/// Returns a list of the spans of the unreachable subpatterns. If `self` is empty (i.e. the
/// whole pattern is unreachable) we return `None`.
fn list_unreachable_spans(&self) -> Option<Vec<Span>> {
/// Panics if `set.is_empty()`.
fn fill_spans(set: &SubPatSet<'_, '_>, spans: &mut Vec<Span>) {
match set {
SubPatSet::Full => bug!(),
SubPatSet::Empty => {}
SubPatSet::Empty => bug!(),
SubPatSet::Full => {}
SubPatSet::Seq { subpats } => {
for (_, sub_set) in subpats {
fill_spans(sub_set, spans);
Expand All @@ -747,8 +761,9 @@ impl<'p, 'tcx> SubPatSet<'p, 'tcx> {
SubPatSet::Alt { subpats, pat, alt_count, .. } => {
let expanded = pat.expand_or_pat();
for i in 0..*alt_count {
let sub_set = subpats.get(&i).unwrap_or(&SubPatSet::Full);
if sub_set.is_full() {
let sub_set = subpats.get(&i).unwrap_or(&SubPatSet::Empty);
if sub_set.is_empty() {
// Found a unreachable subpattern.
spans.push(expanded[i].span);
} else {
fill_spans(sub_set, spans);
Expand All @@ -758,10 +773,11 @@ impl<'p, 'tcx> SubPatSet<'p, 'tcx> {
}
}

if self.is_full() {
if self.is_empty() {
return None;
}
if self.is_empty() {
if self.is_full() {
// No subpatterns are unreachable.
return Some(Vec::new());
}
let mut spans = Vec::new();
Expand Down Expand Up @@ -790,6 +806,7 @@ impl<'p, 'tcx> SubPatSet<'p, 'tcx> {
new_subpats.insert(i - arity + 1, sub_set);
}
}
// If `new_subpats_first_col` has no entries it counts as full, so we can omit it.
if !new_subpats_first_col.is_empty() {
new_subpats.insert(0, Seq { subpats: new_subpats_first_col });
}
Expand All @@ -802,54 +819,58 @@ impl<'p, 'tcx> SubPatSet<'p, 'tcx> {
/// When `self` refers to a patstack that was obtained from splitting an or-pattern, after
/// running `unspecialize` it will refer to the original patstack before splitting.
///
/// This case is subtle. Consider:
/// For example:
/// ```
/// match Some(true) {
/// Some(true) => {}
/// None | Some(true | false) => {}
/// }
/// ```
/// Imagine we naively preserved the sets of unreachable subpatterns. Here `None` would return
/// the empty set and `Some(true | false)` would return the set containing `true`. Intersecting
/// those two would return the empty set, so we'd miss that the last `true` is unreachable.
/// To fix that, when specializing a given alternative of an or-pattern, we consider all other
/// alternatives as unreachable. That way, intersecting the results will not unduly discard
/// unreachable subpatterns coming from the other alternatives. This is what this function does
/// (remember that missing entries in the `Alt` case count as full; in other words alternatives
/// other than `alt_id` count as unreachable).
/// Here `None` would return the full set and `Some(true | false)` would return the set
/// containing `false`. After `unsplit_or_pat`, we want the set to contain `None` and `false`.
/// This is what this function does.
fn unsplit_or_pat(mut self, alt_id: usize, alt_count: usize, pat: &'p Pat<'tcx>) -> Self {
use SubPatSet::*;
if self.is_full() {
return Full;
if self.is_empty() {
return Empty;
}

// Subpatterns coming from inside the or-pattern alternative itself, e.g. in `None | Some(0
// | 1)`.
let set_first_col = match &mut self {
Empty => Empty,
Seq { subpats } => subpats.remove(&0).unwrap_or(Empty),
Full => unreachable!(),
Full => Full,
Seq { subpats } => subpats.remove(&0).unwrap_or(Full),
Empty => unreachable!(),
Alt { .. } => bug!(), // `self` is a patstack
};
let mut subpats_first_col = FxHashMap::default();
subpats_first_col.insert(alt_id, set_first_col);
let set_first_col = Alt { subpats: subpats_first_col, pat, alt_count };

let mut subpats = match self {
Empty => FxHashMap::default(),
Full => FxHashMap::default(),
Seq { subpats } => subpats,
Full => unreachable!(),
Empty => unreachable!(),
Alt { .. } => bug!(), // `self` is a patstack
};
subpats.insert(0, set_first_col);
Seq { subpats }
}
}

/// This carries the results of computing usefulness, as described at the top of the file. When
/// checking usefulness of a match branch, we use the `NoWitnesses` variant, which also keeps track
/// of potential unreachable sub-patterns (in the presence of or-patterns). When checking
/// exhaustiveness of a whole match, we use the `WithWitnesses` variant, which carries a list of
/// witnesses of non-exhaustiveness when there are any.
/// Which variant to use is dictated by `WitnessPreference`.
#[derive(Clone, Debug)]
enum Usefulness<'p, 'tcx> {
/// Carries a set of subpatterns that have been found to be unreachable. If full, this
/// indicates the whole pattern is unreachable. If not, this indicates that the pattern is
/// reachable but has some unreachable sub-patterns (due to or-patterns). In the absence of
/// or-patterns, this is either `Empty` or `Full`.
/// Carries a set of subpatterns that have been found to be reachable. If empty, this indicates
/// the whole pattern is unreachable. If not, this indicates that the pattern is reachable but
/// that some sub-patterns may be unreachable (due to or-patterns). In the absence of
/// or-patterns this will always be either `Empty` (the whole pattern is unreachable) or `Full`
/// (the whole pattern is reachable).
NoWitnesses(SubPatSet<'p, 'tcx>),
/// Carries a list of witnesses of non-exhaustiveness. If empty, indicates that the whole
/// pattern is unreachable.
Expand All @@ -860,13 +881,13 @@ impl<'p, 'tcx> Usefulness<'p, 'tcx> {
fn new_useful(preference: WitnessPreference) -> Self {
match preference {
ConstructWitness => WithWitnesses(vec![Witness(vec![])]),
LeaveOutWitness => NoWitnesses(SubPatSet::empty()),
LeaveOutWitness => NoWitnesses(SubPatSet::full()),
}
}
fn new_not_useful(preference: WitnessPreference) -> Self {
match preference {
ConstructWitness => WithWitnesses(vec![]),
LeaveOutWitness => NoWitnesses(SubPatSet::full()),
LeaveOutWitness => NoWitnesses(SubPatSet::empty()),
}
}

Expand All @@ -876,7 +897,7 @@ impl<'p, 'tcx> Usefulness<'p, 'tcx> {
(WithWitnesses(_), WithWitnesses(o)) if o.is_empty() => {}
(WithWitnesses(s), WithWitnesses(o)) if s.is_empty() => *self = WithWitnesses(o),
(WithWitnesses(s), WithWitnesses(o)) => s.extend(o),
(NoWitnesses(s), NoWitnesses(o)) => s.intersect(o),
(NoWitnesses(s), NoWitnesses(o)) => s.union(o),
_ => unreachable!(),
}
}
Expand All @@ -888,8 +909,8 @@ impl<'p, 'tcx> Usefulness<'p, 'tcx> {
for u in usefulnesses {
ret.extend(u);
if let NoWitnesses(subpats) = &ret {
if subpats.is_empty() {
// Once we reach the empty set, more intersections won't change the result.
if subpats.is_full() {
// Once we reach the full set, more unions won't change the result.
return ret;
}
}
Expand Down Expand Up @@ -1098,8 +1119,7 @@ fn is_useful<'p, 'tcx>(
let v_head = v.head();
let vs: Vec<_> = v.expand_or_pat().collect();
let alt_count = vs.len();
// We expand the or pattern, trying each of its branches in turn and keeping careful track
// of possible unreachable sub-branches.
// We try each or-pattern branch in turn.
let mut matrix = matrix.clone();
let usefulnesses = vs.into_iter().enumerate().map(|(i, v)| {
let usefulness =
Expand Down Expand Up @@ -1155,11 +1175,14 @@ crate struct MatchArm<'p, 'tcx> {
crate has_guard: bool,
}

/// Indicates whether or not a given arm is reachable.
#[derive(Clone, Debug)]
crate enum Reachability {
/// Potentially 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.
/// The arm is reachable. This additionally carries a set of or-pattern branches that have been
/// found to be unreachable despite the overall arm being reachable. Used only in the presence
/// of or-patterns, otherwise it stays empty.
Reachable(Vec<Span>),
/// The arm is unreachable.
Unreachable,
}

Expand Down Expand Up @@ -1195,8 +1218,10 @@ crate fn compute_match_usefulness<'p, 'tcx>(
matrix.push(v);
}
let reachability = match usefulness {
NoWitnesses(subpats) if subpats.is_full() => Reachability::Unreachable,
NoWitnesses(subpats) => Reachability::Reachable(subpats.to_spans().unwrap()),
NoWitnesses(subpats) if subpats.is_empty() => Reachability::Unreachable,
NoWitnesses(subpats) => {
Reachability::Reachable(subpats.list_unreachable_spans().unwrap())
}
WithWitnesses(..) => bug!(),
};
(arm, reachability)
Expand Down

0 comments on commit ae6fcab

Please sign in to comment.