Skip to content

Commit cefcadb

Browse files
committed
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.
1 parent 6319d73 commit cefcadb

File tree

1 file changed

+35
-63
lines changed

1 file changed

+35
-63
lines changed

compiler/rustc_mir_build/src/thir/pattern/usefulness.rs

+35-63
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,8 @@ impl<'p, 'tcx> FromIterator<PatStack<'p, 'tcx>> for Matrix<'p, 'tcx> {
632632
/// empty intersection.
633633
/// It is assumed that two spans don't overlap without one being contained in the other; in other
634634
/// words, that the inclusion structure forms a tree and not a DAG.
635-
/// Operations on this do not need to be fast since it's only nonempty in the diagnostic path.
635+
/// Intersection is not very efficient. It compares everything pairwise. If needed it could be made
636+
/// faster by sorting the `Span`s and merging cleverly.
636637
#[derive(Debug, Clone, Default)]
637638
pub(crate) struct SpanSet {
638639
/// 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> {
715716

716717
/// When trying several branches and each returns a `Usefulness`, we need to combine the
717718
/// results together.
718-
fn merge_or_patterns(usefulnesses: impl Iterator<Item = (Self, Span)>) -> Self {
719+
fn merge(usefulnesses: impl Iterator<Item = Self>) -> Self {
719720
// If we have detected some unreachable sub-branches, we only want to keep them when they
720721
// were unreachable in _all_ branches. Eg. in the following, the last `true` is unreachable
721722
// 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> {
737738
// ```
738739

739740
// Is `None` when no branch was useful. Will often be `Some(Spanset::new())` because the
740-
// sets are only non-empty in the diagnostic path.
741-
let mut unreachables: Option<SpanSet> = None;
742-
// In case of or-patterns we don't want to intersect subpatterns that come from the first
743-
// column. Invariant: contains a list of disjoint spans.
744-
let mut unreachables_this_column = Vec::new();
745-
746-
for (u, branch_span) in usefulnesses {
747-
match u {
748-
Useful(spans) if spans.is_empty() => {
749-
// Hot path: `spans` is only non-empty in the diagnostic path.
750-
unreachables = Some(SpanSet::new());
751-
}
752-
Useful(spans) => {
753-
for span in spans.iter() {
754-
if branch_span.contains(span) {
755-
unreachables_this_column.push(span)
756-
}
757-
}
758-
if let Some(set) = &mut unreachables {
759-
if !set.is_empty() {
760-
set.intersection_mut(&spans);
761-
}
762-
} else {
763-
unreachables = Some(spans);
764-
}
765-
}
766-
NotUseful => unreachables_this_column.push(branch_span),
767-
UsefulWithWitness(_) => {
768-
bug!(
769-
"encountered or-pat in the expansion of `_` during exhaustiveness checking"
770-
)
771-
}
772-
}
773-
}
774-
775-
if let Some(mut unreachables) = unreachables {
776-
for span in unreachables_this_column {
777-
// `unreachables` contained no spans from the first column, and
778-
// `unreachables_this_column` contains only disjoint spans. Therefore it is valid
779-
// to call `push_nonintersecting`.
780-
unreachables.push_nonintersecting(span);
781-
}
782-
Useful(unreachables)
783-
} else {
784-
NotUseful
785-
}
786-
}
787-
788-
/// When trying several branches and each returns a `Usefulness`, we need to combine the
789-
/// results together.
790-
fn merge_split_constructors(usefulnesses: impl Iterator<Item = Self>) -> Self {
791-
// If we have detected some unreachable sub-branches, we only want to keep them when they
792-
// were unreachable in _all_ branches. So we take a big intersection.
793-
794-
// Is `None` when no branch was useful. Will often be `Some(Spanset::new())` because the
795-
// sets are only non-empty in the diagnostic path.
741+
// sets are only non-empty in the presence of or-patterns.
796742
let mut unreachables: Option<SpanSet> = None;
797743
// Witnesses of usefulness, if any.
798744
let mut witnesses = Vec::new();
@@ -831,6 +777,30 @@ impl<'tcx> Usefulness<'tcx> {
831777
}
832778
}
833779

780+
/// After calculating the usefulness for a branch of an or-pattern, call this to make this
781+
/// usefulness mergeable with those from the other branches.
782+
fn unsplit_or_pat(self, this_span: Span, or_pat_spans: &[Span]) -> Self {
783+
match self {
784+
Useful(mut spans) => {
785+
// We register the spans of the other branches of this or-pattern as being
786+
// unreachable from this one. This ensures that intersecting together the sets of
787+
// spans returns what we want.
788+
// Until we optimize `SpanSet` however, intersecting this entails a number of
789+
// comparisons quadratic in the number of branches.
790+
for &span in or_pat_spans {
791+
if span != this_span {
792+
spans.push_nonintersecting(span);
793+
}
794+
}
795+
Useful(spans)
796+
}
797+
x => x,
798+
}
799+
}
800+
801+
/// After calculating usefulness after a specialization, call this to recontruct a usefulness
802+
/// that makes sense for the matrix pre-specialization. This new usefulness can then be merged
803+
/// with the results of specializing with the other constructors.
834804
fn apply_constructor<'p>(
835805
self,
836806
pcx: PatCtxt<'_, 'p, 'tcx>,
@@ -1003,21 +973,23 @@ fn is_useful<'p, 'tcx>(
1003973

1004974
// If the first pattern is an or-pattern, expand it.
1005975
let ret = if let Some(vs) = v.expand_or_pat() {
976+
let subspans: Vec<_> = vs.iter().map(|v| v.head().span).collect();
1006977
// We expand the or pattern, trying each of its branches in turn and keeping careful track
1007978
// of possible unreachable sub-branches.
1008979
let mut matrix = matrix.clone();
1009980
let usefulnesses = vs.into_iter().map(|v| {
1010-
let span = v.head().span;
1011-
let u = is_useful(cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false);
981+
let v_span = v.head().span;
982+
let usefulness =
983+
is_useful(cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false);
1012984
// If pattern has a guard don't add it to the matrix.
1013985
if !is_under_guard {
1014986
// We push the already-seen patterns into the matrix in order to detect redundant
1015987
// branches like `Some(_) | Some(0)`.
1016988
matrix.push(v);
1017989
}
1018-
(u, span)
990+
usefulness.unsplit_or_pat(v_span, &subspans)
1019991
});
1020-
Usefulness::merge_or_patterns(usefulnesses)
992+
Usefulness::merge(usefulnesses)
1021993
} else {
1022994
// We split the head constructor of `v`.
1023995
let ctors = v.head_ctor(cx).split(pcx, Some(hir_id));
@@ -1032,7 +1004,7 @@ fn is_useful<'p, 'tcx>(
10321004
is_useful(pcx.cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false);
10331005
usefulness.apply_constructor(pcx, &ctor, &ctor_wild_subpatterns)
10341006
});
1035-
Usefulness::merge_split_constructors(usefulnesses)
1007+
Usefulness::merge(usefulnesses)
10361008
};
10371009
debug!("is_useful::returns({:#?}, {:#?}) = {:?}", matrix, v, ret);
10381010
ret

0 commit comments

Comments
 (0)