Skip to content

Exhaustiveness: track overlapping ranges precisely #119396

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.

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

Merged
merged 3 commits into from
Jan 12, 2024
Merged
Changes from 1 commit
Commits
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
54 changes: 36 additions & 18 deletions compiler/rustc_pattern_analysis/src/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,7 @@
//! I (Nadrieril) prefer to put new tests in `ui/pattern/usefulness` unless there's a specific
//! reason not to, for example if they crucially depend on a particular feature like `or_patterns`.

use rustc_index::bit_set::BitSet;
use smallvec::{smallvec, SmallVec};
use std::fmt;

Expand Down Expand Up @@ -911,6 +912,11 @@ struct MatrixRow<'p, Cx: TypeCx> {
/// [`compute_exhaustiveness_and_usefulness`] if the arm is found to be useful.
/// This is reset to `false` when specializing.
useful: bool,
/// Tracks which rows above this one have an intersection with this one, i.e. such that there is
/// a value that matches both rows.
/// Note: Because of relevancy we may miss some intersections. The intersections we do find are
/// correct.
intersects: BitSet<usize>,
}

impl<'p, Cx: TypeCx> MatrixRow<'p, Cx> {
Expand Down Expand Up @@ -938,6 +944,7 @@ impl<'p, Cx: TypeCx> MatrixRow<'p, Cx> {
parent_row: self.parent_row,
is_under_guard: self.is_under_guard,
useful: false,
intersects: BitSet::new_empty(0), // Initialized in `Matrix::expand_and_push`.
})
}

Expand All @@ -955,6 +962,7 @@ impl<'p, Cx: TypeCx> MatrixRow<'p, Cx> {
parent_row,
is_under_guard: self.is_under_guard,
useful: false,
intersects: BitSet::new_empty(0), // Initialized in `Matrix::expand_and_push`.
}
}
}
Expand Down Expand Up @@ -993,13 +1001,15 @@ struct Matrix<'p, Cx: TypeCx> {
impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
/// Pushes a new row to the matrix. If the row starts with an or-pattern, this recursively
/// expands it. Internal method, prefer [`Matrix::new`].
fn expand_and_push(&mut self, row: MatrixRow<'p, Cx>) {
fn expand_and_push(&mut self, mut row: MatrixRow<'p, Cx>) {
if !row.is_empty() && row.head().is_or_pat() {
// Expand nested or-patterns.
for new_row in row.expand_or_pat() {
for mut new_row in row.expand_or_pat() {
new_row.intersects = BitSet::new_empty(self.rows.len());
self.rows.push(new_row);
}
Comment on lines +1007 to 1010
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO would be easier to read if it was written like

self
    .rows
    .extend(
        row
            .expand_or_pat()
            .zip(self.rows.len()..)
            .map(|(new_row, len)| Row { intersects: BitSet::new_empty(len), ..new_row }) 
    );

(okay, not sure about the struct update syntax, but mutating the field here is slightly confusing) (also extend ftw)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that the two calls to push are copies of each other, makes it easier to track the invariant that the bitset always has the same length as the number of rows above this one.

} else {
row.intersects = BitSet::new_empty(self.rows.len());
self.rows.push(row);
}
}
Expand All @@ -1019,9 +1029,10 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> {
for (row_id, arm) in arms.iter().enumerate() {
let v = MatrixRow {
pats: PatStack::from_pattern(arm.pat),
parent_row: row_id, // dummy, we won't read it
parent_row: row_id, // dummy, we don't read it
is_under_guard: arm.has_guard,
useful: false,
intersects: BitSet::new_empty(0), // Initialized in `Matrix::expand_and_push`.
};
matrix.expand_and_push(v);
}
Expand Down Expand Up @@ -1349,21 +1360,19 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
let Some(ty) = matrix.head_ty() else {
// The base case: there are no columns in the matrix. We are morally pattern-matching on ().
// A row is useful iff it has no (unguarded) rows above it.
for row in matrix.rows_mut() {
// All rows are useful until they're not.
row.useful = true;
// When there's an unguarded row, the match is exhaustive and any subsequent row is not
// useful.
if !row.is_under_guard {
return Ok(WitnessMatrix::empty());
}
let mut useful = true; // Whether the next row is useful.
for (i, row) in matrix.rows_mut().enumerate() {
row.useful = useful;
row.intersects.insert_range(0..i);
// The next rows stays useful if this one is under a guard.
useful &= row.is_under_guard;
}
// No (unguarded) rows, so the match is not exhaustive. We return a new witness unless
// irrelevant.
return if matrix.wildcard_row_is_relevant {
return if useful && matrix.wildcard_row_is_relevant {
// The wildcard row is useful; the match is non-exhaustive.
Ok(WitnessMatrix::unit_witness())
} else {
// We choose to not report anything here; see at the top for details.
// Either the match is exhaustive, or we choose not to report anything because of
// relevancy. See at the top for details.
Ok(WitnessMatrix::empty())
};
};
Expand Down Expand Up @@ -1424,10 +1433,19 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
// Accumulate the found witnesses.
ret.extend(witnesses);

// A parent row is useful if any of its children is.
for child_row in spec_matrix.rows() {
let parent_row = &mut matrix.rows[child_row.parent_row];
parent_row.useful = parent_row.useful || child_row.useful;
let parent_row_id = child_row.parent_row;
let parent_row = &mut matrix.rows[parent_row_id];
// A parent row is useful if any of its children is.
parent_row.useful |= child_row.useful;
for child_intersection in child_row.intersects.iter() {
// Convert the intersecting ids into ids for the parent matrix.
let parent_intersection = spec_matrix.rows[child_intersection].parent_row;
// Note: self-intersection can happen with or-patterns.
if parent_intersection != parent_row_id {
parent_row.intersects.insert(parent_intersection);
}
}
}
}

Expand Down