Skip to content

Commit

Permalink
Auto merge of #120324 - Nadrieril:remove-interior-mutability, r=<try>
Browse files Browse the repository at this point in the history
Experiment: track usefulness without interior mutability

This would remove a potential footgun in r-a. Just wanna see if this kills perf
  • Loading branch information
bors committed Jan 24, 2024
2 parents 7ffc697 + 8f04a1e commit 09d4a12
Show file tree
Hide file tree
Showing 7 changed files with 246 additions and 228 deletions.
26 changes: 12 additions & 14 deletions compiler/rustc_pattern_analysis/src/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ use self::MaybeInfiniteInt::*;
use self::SliceKind::*;

use crate::index;
use crate::usefulness::PlaceCtxt;
use crate::TypeCx;

/// Whether we have seen a constructor in the column or not.
Expand Down Expand Up @@ -718,21 +717,20 @@ impl<Cx: TypeCx> Constructor<Cx> {

/// The number of fields for this constructor. This must be kept in sync with
/// `Fields::wildcards`.
pub(crate) fn arity(&self, pcx: &PlaceCtxt<'_, Cx>) -> usize {
pcx.ctor_arity(self)
pub(crate) fn arity(&self, cx: &Cx, ty: &Cx::Ty) -> usize {
cx.ctor_arity(self, ty)
}

/// Returns whether `self` is covered by `other`, i.e. whether `self` is a subset of `other`.
/// For the simple cases, this is simply checking for equality. For the "grouped" constructors,
/// this checks for inclusion.
// We inline because this has a single call site in `Matrix::specialize_constructor`.
#[inline]
pub(crate) fn is_covered_by(&self, pcx: &PlaceCtxt<'_, Cx>, other: &Self) -> bool {
pub(crate) fn is_covered_by(&self, cx: &Cx, other: &Self) -> bool {
match (self, other) {
(Wildcard, _) => pcx
.mcx
.tycx
.bug(format_args!("Constructor splitting should not have returned `Wildcard`")),
(Wildcard, _) => {
cx.bug(format_args!("Constructor splitting should not have returned `Wildcard`"))
}
// Wildcards cover anything
(_, Wildcard) => true,
// Only a wildcard pattern can match these special constructors.
Expand Down Expand Up @@ -773,7 +771,7 @@ impl<Cx: TypeCx> Constructor<Cx> {
(Opaque(self_id), Opaque(other_id)) => self_id == other_id,
(Opaque(..), _) | (_, Opaque(..)) => false,

_ => pcx.mcx.tycx.bug(format_args!(
_ => cx.bug(format_args!(
"trying to compare incompatible constructors {self:?} and {other:?}"
)),
}
Expand Down Expand Up @@ -850,10 +848,10 @@ pub enum ConstructorSet<Cx: TypeCx> {
/// of the `ConstructorSet` for the type, yet if we forgot to include them in `present` we would be
/// ignoring any row with `Opaque`s in the algorithm. Hence the importance of point 4.
#[derive(Debug)]
pub(crate) struct SplitConstructorSet<Cx: TypeCx> {
pub(crate) present: SmallVec<[Constructor<Cx>; 1]>,
pub(crate) missing: Vec<Constructor<Cx>>,
pub(crate) missing_empty: Vec<Constructor<Cx>>,
pub struct SplitConstructorSet<Cx: TypeCx> {
pub present: SmallVec<[Constructor<Cx>; 1]>,
pub missing: Vec<Constructor<Cx>>,
pub missing_empty: Vec<Constructor<Cx>>,
}

impl<Cx: TypeCx> ConstructorSet<Cx> {
Expand All @@ -862,7 +860,7 @@ impl<Cx: TypeCx> ConstructorSet<Cx> {
/// or slices. This can get subtle; see [`SplitConstructorSet`] for details of this operation
/// and its invariants.
#[instrument(level = "debug", skip(self, ctors), ret)]
pub(crate) fn split<'a>(
pub fn split<'a>(
&self,
ctors: impl Iterator<Item = &'a Constructor<Cx>> + Clone,
) -> SplitConstructorSet<Cx>
Expand Down
18 changes: 5 additions & 13 deletions compiler/rustc_pattern_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub mod errors;
#[cfg(feature = "rustc")]
pub(crate) mod lints;
pub mod pat;
pub mod pat_column;
#[cfg(feature = "rustc")]
pub mod rustc;
pub mod usefulness;
Expand Down Expand Up @@ -67,8 +68,9 @@ use rustc_span::ErrorGuaranteed;

use crate::constructor::{Constructor, ConstructorSet, IntRange};
#[cfg(feature = "rustc")]
use crate::lints::{lint_nonexhaustive_missing_variants, PatternColumn};
use crate::lints::lint_nonexhaustive_missing_variants;
use crate::pat::DeconstructedPat;
use crate::pat_column::PatternColumn;
#[cfg(feature = "rustc")]
use crate::rustc::RustcMatchCheckCtxt;
#[cfg(feature = "rustc")]
Expand Down Expand Up @@ -126,14 +128,6 @@ pub trait TypeCx: Sized + fmt::Debug {
}
}

/// Context that provides information global to a match.
#[derive(derivative::Derivative)]
#[derivative(Clone(bound = ""), Copy(bound = ""))]
pub struct MatchCtxt<'a, Cx: TypeCx> {
/// The context for type information.
pub tycx: &'a Cx,
}

/// The arm of a match expression.
#[derive(Debug)]
#[derive(derivative::Derivative)]
Expand All @@ -154,15 +148,13 @@ pub fn analyze_match<'p, 'tcx>(
) -> Result<rustc::UsefulnessReport<'p, 'tcx>, ErrorGuaranteed> {
let scrut_ty = tycx.reveal_opaque_ty(scrut_ty);
let scrut_validity = ValidityConstraint::from_bool(tycx.known_valid_scrutinee);
let cx = MatchCtxt { tycx };

let report = compute_match_usefulness(cx, arms, scrut_ty, scrut_validity)?;
let report = compute_match_usefulness(tycx, arms, scrut_ty, scrut_validity)?;

// Run the non_exhaustive_omitted_patterns lint. Only run on refutable patterns to avoid hitting
// `if let`s. Only run if the match is exhaustive otherwise the error is redundant.
if tycx.refutable && report.non_exhaustiveness_witnesses.is_empty() {
let pat_column = PatternColumn::new(arms);
lint_nonexhaustive_missing_variants(cx, arms, &pat_column, scrut_ty)?;
lint_nonexhaustive_missing_variants(tycx, arms, &pat_column, scrut_ty)?;
}

Ok(report)
Expand Down
120 changes: 17 additions & 103 deletions compiler/rustc_pattern_analysis/src/lints.rs
Original file line number Diff line number Diff line change
@@ -1,109 +1,24 @@
use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS;
use rustc_span::ErrorGuaranteed;

use crate::constructor::Constructor;
use crate::errors::{NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered};
use crate::pat::PatOrWild;
use crate::rustc::{
Constructor, DeconstructedPat, MatchArm, MatchCtxt, PlaceCtxt, RevealedTy, RustcMatchCheckCtxt,
SplitConstructorSet, WitnessPat,
};

/// A column of patterns in the matrix, where a column is the intuitive notion of "subpatterns that
/// inspect the same subvalue/place".
/// This is used to traverse patterns column-by-column for lints. Despite similarities with the
/// algorithm in [`crate::usefulness`], this does a different traversal. Notably this is linear in
/// the depth of patterns, whereas `compute_exhaustiveness_and_usefulness` is worst-case exponential
/// (exhaustiveness is NP-complete). The core difference is that we treat sub-columns separately.
///
/// This must not contain an or-pattern. `expand_and_push` takes care to expand them.
///
/// This is not used in the usefulness algorithm; only in lints.
#[derive(Debug)]
pub(crate) struct PatternColumn<'p, 'tcx> {
patterns: Vec<&'p DeconstructedPat<'p, 'tcx>>,
}

impl<'p, 'tcx> PatternColumn<'p, 'tcx> {
pub(crate) fn new(arms: &[MatchArm<'p, 'tcx>]) -> Self {
let patterns = Vec::with_capacity(arms.len());
let mut column = PatternColumn { patterns };
for arm in arms {
column.expand_and_push(PatOrWild::Pat(arm.pat));
}
column
}
/// Pushes a pattern onto the column, expanding any or-patterns into its subpatterns.
/// Internal method, prefer [`PatternColumn::new`].
fn expand_and_push(&mut self, pat: PatOrWild<'p, RustcMatchCheckCtxt<'p, 'tcx>>) {
// We flatten or-patterns and skip algorithm-generated wildcards.
if pat.is_or_pat() {
self.patterns.extend(
pat.flatten_or_pat().into_iter().filter_map(|pat_or_wild| pat_or_wild.as_pat()),
)
} else if let Some(pat) = pat.as_pat() {
self.patterns.push(pat)
}
}

fn head_ty(&self) -> Option<RevealedTy<'tcx>> {
self.patterns.first().map(|pat| *pat.ty())
}

/// Do constructor splitting on the constructors of the column.
fn analyze_ctors(
&self,
pcx: &PlaceCtxt<'_, 'p, 'tcx>,
) -> Result<SplitConstructorSet<'p, 'tcx>, ErrorGuaranteed> {
let column_ctors = self.patterns.iter().map(|p| p.ctor());
let ctors_for_ty = &pcx.ctors_for_ty()?;
Ok(ctors_for_ty.split(column_ctors))
}

/// Does specialization: given a constructor, this takes the patterns from the column that match
/// the constructor, and outputs their fields.
/// This returns one column per field of the constructor. They usually all have the same length
/// (the number of patterns in `self` that matched `ctor`), except that we expand or-patterns
/// which may change the lengths.
fn specialize(
&self,
pcx: &PlaceCtxt<'_, 'p, 'tcx>,
ctor: &Constructor<'p, 'tcx>,
) -> Vec<PatternColumn<'p, 'tcx>> {
let arity = ctor.arity(pcx);
if arity == 0 {
return Vec::new();
}

// We specialize the column by `ctor`. This gives us `arity`-many columns of patterns. These
// columns may have different lengths in the presence of or-patterns (this is why we can't
// reuse `Matrix`).
let mut specialized_columns: Vec<_> =
(0..arity).map(|_| Self { patterns: Vec::new() }).collect();
let relevant_patterns =
self.patterns.iter().filter(|pat| ctor.is_covered_by(pcx, pat.ctor()));
for pat in relevant_patterns {
let specialized = pat.specialize(ctor, arity);
for (subpat, column) in specialized.into_iter().zip(&mut specialized_columns) {
column.expand_and_push(subpat);
}
}
specialized_columns
}
}
use crate::pat_column::PatternColumn;
use crate::rustc::{RevealedTy, RustcMatchCheckCtxt, WitnessPat};
use crate::MatchArm;

/// Traverse the patterns to collect any variants of a non_exhaustive enum that fail to be mentioned
/// in a given column.
#[instrument(level = "debug", skip(cx), ret)]
fn collect_nonexhaustive_missing_variants<'a, 'p, 'tcx>(
cx: MatchCtxt<'a, 'p, 'tcx>,
column: &PatternColumn<'p, 'tcx>,
cx: &RustcMatchCheckCtxt<'p, 'tcx>,
column: &PatternColumn<'p, RustcMatchCheckCtxt<'p, 'tcx>>,
) -> Result<Vec<WitnessPat<'p, 'tcx>>, ErrorGuaranteed> {
let Some(ty) = column.head_ty() else {
let Some(&ty) = column.head_ty() else {
return Ok(Vec::new());
};
let pcx = &PlaceCtxt::new_dummy(cx, &ty);

let set = column.analyze_ctors(pcx)?;
let set = column.analyze_ctors(cx, &ty)?;
if set.present.is_empty() {
// We can't consistently handle the case where no constructors are present (since this would
// require digging deep through any type in case there's a non_exhaustive enum somewhere),
Expand All @@ -112,20 +27,20 @@ fn collect_nonexhaustive_missing_variants<'a, 'p, 'tcx>(
}

let mut witnesses = Vec::new();
if cx.tycx.is_foreign_non_exhaustive_enum(ty) {
if cx.is_foreign_non_exhaustive_enum(ty) {
witnesses.extend(
set.missing
.into_iter()
// This will list missing visible variants.
.filter(|c| !matches!(c, Constructor::Hidden | Constructor::NonExhaustive))
.map(|missing_ctor| WitnessPat::wild_from_ctor(pcx, missing_ctor)),
.map(|missing_ctor| WitnessPat::wild_from_ctor(cx, missing_ctor, ty)),
)
}

// Recurse into the fields.
for ctor in set.present {
let specialized_columns = column.specialize(pcx, &ctor);
let wild_pat = WitnessPat::wild_from_ctor(pcx, ctor);
let specialized_columns = column.specialize(cx, &ty, &ctor);
let wild_pat = WitnessPat::wild_from_ctor(cx, ctor, ty);
for (i, col_i) in specialized_columns.iter().enumerate() {
// Compute witnesses for each column.
let wits_for_col_i = collect_nonexhaustive_missing_variants(cx, col_i)?;
Expand All @@ -141,18 +56,17 @@ fn collect_nonexhaustive_missing_variants<'a, 'p, 'tcx>(
Ok(witnesses)
}

pub(crate) fn lint_nonexhaustive_missing_variants<'a, 'p, 'tcx>(
cx: MatchCtxt<'a, 'p, 'tcx>,
arms: &[MatchArm<'p, 'tcx>],
pat_column: &PatternColumn<'p, 'tcx>,
pub(crate) fn lint_nonexhaustive_missing_variants<'p, 'tcx>(
rcx: &RustcMatchCheckCtxt<'p, 'tcx>,
arms: &[MatchArm<'p, RustcMatchCheckCtxt<'p, 'tcx>>],
pat_column: &PatternColumn<'p, RustcMatchCheckCtxt<'p, 'tcx>>,
scrut_ty: RevealedTy<'tcx>,
) -> Result<(), ErrorGuaranteed> {
let rcx: &RustcMatchCheckCtxt<'_, '_> = cx.tycx;
if !matches!(
rcx.tcx.lint_level_at_node(NON_EXHAUSTIVE_OMITTED_PATTERNS, rcx.match_lint_level).0,
rustc_session::lint::Level::Allow
) {
let witnesses = collect_nonexhaustive_missing_variants(cx, pat_column)?;
let witnesses = collect_nonexhaustive_missing_variants(rcx, pat_column)?;
if !witnesses.is_empty() {
// Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns`
// is not exhaustive enough.
Expand Down

0 comments on commit 09d4a12

Please sign in to comment.