Skip to content

Commit

Permalink
Track redundant subpatterns without interior mutability
Browse files Browse the repository at this point in the history
  • Loading branch information
Nadrieril committed Jan 24, 2024
1 parent de13a09 commit 71208c5
Showing 1 changed file with 52 additions and 15 deletions.
67 changes: 52 additions & 15 deletions compiler/rustc_pattern_analysis/src/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -712,9 +712,11 @@
//! 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_hash::FxHashSet;
use rustc_index::bit_set::BitSet;
use smallvec::{smallvec, SmallVec};
use std::fmt;
use std::ops::Deref;

use crate::constructor::{Constructor, ConstructorSet, IntRange};
use crate::pat::{DeconstructedPat, PatOrWild, WitnessPat};
Expand All @@ -729,12 +731,36 @@ pub fn ensure_sufficient_stack<R>(f: impl FnOnce() -> R) -> R {
f()
}

/// Wrapper type for by-address hashing. Comparison and hashing of the wrapped pointer type will be
/// based on the address of its contents, rather than their value.
struct ByAddress<T>(T);

impl<T: Deref> ByAddress<T> {
fn addr(&self) -> *const T::Target {
(&*self.0) as *const _
}
}
/// Raw pointer hashing and comparison.
impl<T: Deref> std::hash::Hash for ByAddress<T> {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.addr().hash(state)
}
}
impl<T: Deref> PartialEq for ByAddress<T> {
fn eq(&self, other: &Self) -> bool {
std::ptr::eq(self.addr(), other.addr())
}
}
impl<T: Deref> Eq for ByAddress<T> {}

/// Context that provides information for usefulness checking.
#[derive(derivative::Derivative)]
#[derivative(Clone(bound = ""), Copy(bound = ""))]
pub struct UsefulnessCtxt<'a, Cx: TypeCx> {
struct UsefulnessCtxt<'a, 'p, Cx: TypeCx> {
/// The context for type information.
pub tycx: &'a Cx,
tycx: &'a Cx,
/// Collect the patterns found useful during usefulness checking. This is used to lint
/// unreachable (sub)patterns. We distinguish patterns by their address to avoid needing to
/// inspect the contents. They'll all be distinct anyway since they carry a `Span`.
useful_subpatterns: FxHashSet<ByAddress<&'p DeconstructedPat<'p, Cx>>>,
}

/// Context that provides information local to a place under investigation.
Expand Down Expand Up @@ -1330,7 +1356,7 @@ impl<Cx: TypeCx> WitnessMatrix<Cx> {
/// We can however get false negatives because exhaustiveness does not explore all cases. See the
/// section on relevancy at the top of the file.
fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
mcx: UsefulnessCtxt<'_, Cx>,
mcx: &mut UsefulnessCtxt<'_, 'p, Cx>,
overlap_range: IntRange,
matrix: &Matrix<'p, Cx>,
specialized_matrix: &Matrix<'p, Cx>,
Expand Down Expand Up @@ -1403,7 +1429,7 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
/// This is all explained at the top of the file.
#[instrument(level = "debug", skip(mcx, is_top_level), ret)]
fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
mcx: UsefulnessCtxt<'a, Cx>,
mcx: &mut UsefulnessCtxt<'a, 'p, Cx>,
matrix: &mut Matrix<'p, Cx>,
is_top_level: bool,
) -> Result<WitnessMatrix<Cx>, Cx::Error> {
Expand Down Expand Up @@ -1524,7 +1550,9 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
// Record usefulness in the patterns.
for row in matrix.rows() {
if row.useful {
row.head().set_useful();
if let PatOrWild::Pat(pat) = row.head() {
mcx.useful_subpatterns.insert(ByAddress(pat));
}
}
}

Expand All @@ -1545,12 +1573,17 @@ pub enum Usefulness<'p, Cx: TypeCx> {

/// Report whether this pattern was found useful, and its subpatterns that were not useful if any.
fn collect_pattern_usefulness<'p, Cx: TypeCx>(
useful_subpatterns: &FxHashSet<ByAddress<&'p DeconstructedPat<'p, Cx>>>,
pat: &'p DeconstructedPat<'p, Cx>,
) -> Usefulness<'p, Cx> {
fn pat_is_useful<'p, Cx: TypeCx>(pat: &'p DeconstructedPat<'p, Cx>) -> bool {
if pat.useful.get() {
fn pat_is_useful<'p, Cx: TypeCx>(
useful_subpatterns: &FxHashSet<ByAddress<&'p DeconstructedPat<'p, Cx>>>,
pat: &'p DeconstructedPat<'p, Cx>,
) -> bool {
if useful_subpatterns.contains(&ByAddress(pat)) {
true
} else if pat.is_or_pat() && pat.iter_fields().any(|f| pat_is_useful(f)) {
} else if pat.is_or_pat() && pat.iter_fields().any(|f| pat_is_useful(useful_subpatterns, f))
{
// We always expand or patterns in the matrix, so we will never see the actual
// or-pattern (the one with constructor `Or`) in the column. As such, it will not be
// marked as useful itself, only its children will. We recover this information here.
Expand All @@ -1562,7 +1595,7 @@ fn collect_pattern_usefulness<'p, Cx: TypeCx>(

let mut subpats = Vec::new();
pat.walk(&mut |p| {
if pat_is_useful(p) {
if pat_is_useful(useful_subpatterns, p) {
// The pattern is useful, so we recurse to find redundant subpatterns.
true
} else {
Expand All @@ -1572,7 +1605,11 @@ fn collect_pattern_usefulness<'p, Cx: TypeCx>(
}
});

if pat_is_useful(pat) { Usefulness::Useful(subpats) } else { Usefulness::Redundant }
if pat_is_useful(useful_subpatterns, pat) {
Usefulness::Useful(subpats)
} else {
Usefulness::Redundant
}
}

/// The output of checking a match for exhaustiveness and arm usefulness.
Expand All @@ -1592,18 +1629,18 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>(
scrut_ty: Cx::Ty,
scrut_validity: ValidityConstraint,
) -> Result<UsefulnessReport<'p, Cx>, Cx::Error> {
let cx = UsefulnessCtxt { tycx };
let mut cx = UsefulnessCtxt { tycx, useful_subpatterns: FxHashSet::default() };
let mut matrix = Matrix::new(arms, scrut_ty, scrut_validity);
let non_exhaustiveness_witnesses =
compute_exhaustiveness_and_usefulness(cx, &mut matrix, true)?;
compute_exhaustiveness_and_usefulness(&mut cx, &mut matrix, true)?;

let non_exhaustiveness_witnesses: Vec<_> = non_exhaustiveness_witnesses.single_column();
let arm_usefulness: Vec<_> = arms
.iter()
.copied()
.map(|arm| {
debug!(?arm);
let usefulness = collect_pattern_usefulness(arm.pat);
let usefulness = collect_pattern_usefulness(&cx.useful_subpatterns, arm.pat);
(arm, usefulness)
})
.collect();
Expand Down

0 comments on commit 71208c5

Please sign in to comment.