mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #120324 - Nadrieril:remove-interior-mutability, r=compiler-errors

Browse Source 
pattern_analysis: track usefulness without interior mutability

Because of or-patterns, exhaustiveness needs to be able to lint if a sub-pattern is redundant, e.g. in `Some(_) | Some(true)`. So far the only sane solution I had found was interior mutability. This is a bit of an abstraction leak, and would become a footgun if we ever reused the same `DeconstructedPat`. This PR replaces interior mutability with an address-indexed hashmap, which is logically equivalent.
This commit is contained in:
bors 2024-02-12 22:16:58 +00:00
commit 74c3f5a146
2 changed files with 86 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
compiler/rustc_pattern_analysis/src/pat.rs
View File

@ -1,6 +1,5 @@
//! As explained in [`crate::usefulness`], values and patterns are made from constructors applied to
//! fields. This file defines types that represent patterns in this way.
use std::cell::Cell;
use std::fmt;
use smallvec::{smallvec, SmallVec};
@ -10,12 +9,20 @@ use crate::TypeCx;
use self::Constructor::*;
/// A globally unique id to distinguish patterns.
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub(crate) struct PatId(u32);
impl PatId {
fn new() -> Self {
use std::sync::atomic::{AtomicU32, Ordering};
static PAT_ID: AtomicU32 = AtomicU32::new(0);
PatId(PAT_ID.fetch_add(1, Ordering::SeqCst))
}
}
/// Values and patterns can be represented as a constructor applied to some fields. This represents
/// a pattern in this form.
/// This also uses interior mutability to keep track of whether the pattern has been found reachable
/// during analysis. For this reason they cannot be cloned.
/// A `DeconstructedPat` will almost always come from user input; the only exception are some
/// `Wildcard`s introduced during specialization.
/// a pattern in this form. A `DeconstructedPat` will almost always come from user input; the only
/// exception are some `Wildcard`s introduced during pattern lowering.
///
/// Note that the number of fields may not match the fields declared in the original struct/variant.
/// This happens if a private or `non_exhaustive` field is uninhabited, because the code mustn't
@ -28,19 +35,13 @@ pub struct DeconstructedPat<Cx: TypeCx> {
/// Extra data to store in a pattern. `None` if the pattern is a wildcard that does not
/// correspond to a user-supplied pattern.
data: Option<Cx::PatData>,
/// Whether removing this arm would change the behavior of the match expression.
useful: Cell<bool>,
/// Globally-unique id used to track usefulness at the level of subpatterns.
pub(crate) uid: PatId,
}
impl<Cx: TypeCx> DeconstructedPat<Cx> {
pub fn wildcard(ty: Cx::Ty) -> Self {
DeconstructedPat {
ctor: Wildcard,
fields: Vec::new(),
ty,
data: None,
useful: Cell::new(false),
}
DeconstructedPat { ctor: Wildcard, fields: Vec::new(), ty, data: None, uid: PatId::new() }
}
pub fn new(
@ -49,7 +50,7 @@ impl<Cx: TypeCx> DeconstructedPat<Cx> {
ty: Cx::Ty,
data: Cx::PatData,
) -> Self {
DeconstructedPat { ctor, fields, ty, data: Some(data), useful: Cell::new(false) }
DeconstructedPat { ctor, fields, ty, data: Some(data), uid: PatId::new() }
}
pub(crate) fn is_or_pat(&self) -> bool {
@ -107,39 +108,16 @@ impl<Cx: TypeCx> DeconstructedPat<Cx> {
}
}
/// We keep track for each pattern if it was ever useful during the analysis. This is used with
/// `redundant_subpatterns` to report redundant subpatterns arising from or patterns.
pub(crate) fn set_useful(&self) {
self.useful.set(true)
}
pub(crate) fn is_useful(&self) -> bool {
if self.useful.get() {
true
} else if self.is_or_pat() && self.iter_fields().any(|f| f.is_useful()) {
// 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.
self.set_useful();
true
} else {
false
/// Walk top-down and call `it` in each place where a pattern occurs
/// starting with the root pattern `walk` is called on. If `it` returns
/// false then we will descend no further but siblings will be processed.
pub fn walk<'a>(&'a self, it: &mut impl FnMut(&'a Self) -> bool) {
if !it(self) {
return;
}
}
/// Report the subpatterns that were not useful, if any.
pub(crate) fn redundant_subpatterns(&self) -> Vec<&Self> {
let mut subpats = Vec::new();
self.collect_redundant_subpatterns(&mut subpats);
subpats
}
fn collect_redundant_subpatterns<'a>(&'a self, subpats: &mut Vec<&'a Self>) {
// We don't look at subpatterns if we already reported the whole pattern as redundant.
if !self.is_useful() {
subpats.push(self);
} else {
for p in self.iter_fields() {
p.collect_redundant_subpatterns(subpats);
}
for p in self.iter_fields() {
p.walk(it)
}
}
}
@ -284,12 +262,6 @@ impl<'p, Cx: TypeCx> PatOrWild<'p, Cx> {
PatOrWild::Pat(pat) => pat.specialize(other_ctor, ctor_arity),
}
}
pub(crate) fn set_useful(&self) {
if let PatOrWild::Pat(pat) = self {
pat.set_useful()
}
}
}
impl<'p, Cx: TypeCx> fmt::Debug for PatOrWild<'p, Cx> {

91
compiler/rustc_pattern_analysis/src/usefulness.rs
View File

@ -466,13 +466,9 @@
//! first pattern of a row in the matrix is an or-pattern, we expand it by duplicating the rest of
//! the row as necessary. This is handled automatically in [`Matrix`].
//!
//! This makes usefulness tracking subtle, because we also want to compute whether an alternative
//! of an or-pattern is redundant, e.g. in `Some(_) | Some(0)`. We track usefulness of each
//! subpattern by interior mutability in [`DeconstructedPat`] with `set_useful`/`is_useful`.
//!
//! It's unfortunate that we have to use interior mutability, but believe me (Nadrieril), I have
//! tried [other](https://github.com/rust-lang/rust/pull/80104)
//! [solutions](https://github.com/rust-lang/rust/pull/80632) and nothing is remotely as simple.
//! This makes usefulness tracking subtle, because we also want to compute whether an alternative of
//! an or-pattern is redundant, e.g. in `Some(_) | Some(0)`. We therefore track usefulness of each
//! subpattern of the match.
//!
//!
//!
@ -713,12 +709,13 @@
//! 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 crate::constructor::{Constructor, ConstructorSet, IntRange};
use crate::pat::{DeconstructedPat, PatOrWild, WitnessPat};
use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat};
use crate::{Captures, MatchArm, TypeCx};
use self::ValidityConstraint::*;
@ -731,16 +728,12 @@ pub fn ensure_sufficient_stack<R>(f: impl FnOnce() -> R) -> R {
}
/// Context that provides information for usefulness checking.
pub struct UsefulnessCtxt<'a, Cx: TypeCx> {
struct UsefulnessCtxt<'a, Cx: TypeCx> {
/// The context for type information.
pub tycx: &'a Cx,
}
impl<'a, Cx: TypeCx> Copy for UsefulnessCtxt<'a, Cx> {}
impl<'a, Cx: TypeCx> Clone for UsefulnessCtxt<'a, Cx> {
fn clone(&self) -> Self {
Self { tycx: self.tycx }
}
tycx: &'a Cx,
/// Collect the patterns found useful during usefulness checking. This is used to lint
/// unreachable (sub)patterns.
useful_subpatterns: FxHashSet<PatId>,
}
/// Context that provides information local to a place under investigation.
@ -1381,7 +1374,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<'_, Cx>,
overlap_range: IntRange,
matrix: &Matrix<'p, Cx>,
specialized_matrix: &Matrix<'p, Cx>,
@ -1441,8 +1434,8 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
/// The core of the algorithm.
///
/// This recursively computes witnesses of the non-exhaustiveness of `matrix` (if any). Also tracks
/// usefulness of each row in the matrix (in `row.useful`). We track usefulness of each
/// subpattern using interior mutability in `DeconstructedPat`.
/// usefulness of each row in the matrix (in `row.useful`). We track usefulness of each subpattern
/// in `mcx.useful_subpatterns`.
///
/// The input `Matrix` and the output `WitnessMatrix` together match the type exhaustively.
///
@ -1454,7 +1447,7 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
/// This is all explained at the top of the file.
#[instrument(level = "debug", skip(mcx), ret)]
fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
mcx: UsefulnessCtxt<'a, Cx>,
mcx: &mut UsefulnessCtxt<'a, Cx>,
matrix: &mut Matrix<'p, Cx>,
) -> Result<WitnessMatrix<Cx>, Cx::Error> {
debug_assert!(matrix.rows().all(|r| r.len() == matrix.column_count()));
@ -1578,7 +1571,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(pat.uid);
}
}
}
@ -1597,6 +1592,47 @@ pub enum Usefulness<'p, Cx: TypeCx> {
Redundant,
}
/// 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<PatId>,
pat: &'p DeconstructedPat<Cx>,
) -> Usefulness<'p, Cx> {
fn pat_is_useful<'p, Cx: TypeCx>(
useful_subpatterns: &FxHashSet<PatId>,
pat: &'p DeconstructedPat<Cx>,
) -> bool {
if useful_subpatterns.contains(&pat.uid) {
true
} 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.
true
} else {
false
}
}
let mut redundant_subpats = Vec::new();
pat.walk(&mut |p| {
if pat_is_useful(useful_subpatterns, p) {
// The pattern is useful, so we recurse to find redundant subpatterns.
true
} else {
// The pattern is redundant.
redundant_subpats.push(p);
false // stop recursing
}
});
if pat_is_useful(useful_subpatterns, pat) {
Usefulness::Useful(redundant_subpats)
} else {
Usefulness::Redundant
}
}
/// The output of checking a match for exhaustiveness and arm usefulness.
pub struct UsefulnessReport<'p, Cx: TypeCx> {
/// For each arm of the input, whether that arm is useful after the arms above it.
@ -1614,9 +1650,9 @@ 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)?;
let non_exhaustiveness_witnesses = compute_exhaustiveness_and_usefulness(&mut cx, &mut matrix)?;
let non_exhaustiveness_witnesses: Vec<_> = non_exhaustiveness_witnesses.single_column();
let arm_usefulness: Vec<_> = arms
@ -1624,12 +1660,7 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>(
.copied()
.map(|arm| {
debug!(?arm);
// We warn when a pattern is not useful.
let usefulness = if arm.pat.is_useful() {
Usefulness::Useful(arm.pat.redundant_subpatterns())
} else {
Usefulness::Redundant
};
let usefulness = collect_pattern_usefulness(&cx.useful_subpatterns, arm.pat);
(arm, usefulness)
})
.collect();