From 0b2579a1b617e83da98008bac9b93c36c43a8f4b Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 24 Jan 2024 20:38:21 +0100 Subject: [PATCH 1/5] Make `PatternColumn` generic in `Cx` --- compiler/rustc_pattern_analysis/src/lints.rs | 43 +++++++++----------- compiler/rustc_pattern_analysis/src/rustc.rs | 4 -- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/lints.rs b/compiler/rustc_pattern_analysis/src/lints.rs index 4bfe7dfb072c4..e3d218d71c301 100644 --- a/compiler/rustc_pattern_analysis/src/lints.rs +++ b/compiler/rustc_pattern_analysis/src/lints.rs @@ -1,12 +1,12 @@ use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS; use rustc_span::ErrorGuaranteed; +use crate::constructor::{Constructor, SplitConstructorSet}; use crate::errors::{NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered}; -use crate::pat::PatOrWild; -use crate::rustc::{ - Constructor, DeconstructedPat, MatchArm, MatchCtxt, PlaceCtxt, RevealedTy, RustcMatchCheckCtxt, - SplitConstructorSet, WitnessPat, -}; +use crate::pat::{DeconstructedPat, PatOrWild}; +use crate::rustc::{MatchCtxt, RevealedTy, RustcMatchCheckCtxt, WitnessPat}; +use crate::usefulness::PlaceCtxt; +use crate::{MatchArm, TypeCx}; /// A column of patterns in the matrix, where a column is the intuitive notion of "subpatterns that /// inspect the same subvalue/place". @@ -19,12 +19,12 @@ use crate::rustc::{ /// /// 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>>, +pub(crate) struct PatternColumn<'p, Cx: TypeCx> { + patterns: Vec<&'p DeconstructedPat<'p, Cx>>, } -impl<'p, 'tcx> PatternColumn<'p, 'tcx> { - pub(crate) fn new(arms: &[MatchArm<'p, 'tcx>]) -> Self { +impl<'p, Cx: TypeCx> PatternColumn<'p, Cx> { + pub(crate) fn new(arms: &[MatchArm<'p, Cx>]) -> Self { let patterns = Vec::with_capacity(arms.len()); let mut column = PatternColumn { patterns }; for arm in arms { @@ -34,7 +34,7 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> { } /// 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>>) { + fn expand_and_push(&mut self, pat: PatOrWild<'p, Cx>) { // We flatten or-patterns and skip algorithm-generated wildcards. if pat.is_or_pat() { self.patterns.extend( @@ -45,15 +45,12 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> { } } - fn head_ty(&self) -> Option> { - self.patterns.first().map(|pat| *pat.ty()) + fn head_ty(&self) -> Option<&Cx::Ty> { + 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, ErrorGuaranteed> { + fn analyze_ctors(&self, pcx: &PlaceCtxt<'_, Cx>) -> Result, Cx::Error> { 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)) @@ -66,9 +63,9 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> { /// which may change the lengths. fn specialize( &self, - pcx: &PlaceCtxt<'_, 'p, 'tcx>, - ctor: &Constructor<'p, 'tcx>, - ) -> Vec> { + pcx: &PlaceCtxt<'_, Cx>, + ctor: &Constructor, + ) -> Vec> { let arity = ctor.arity(pcx); if arity == 0 { return Vec::new(); @@ -96,9 +93,9 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> { #[instrument(level = "debug", skip(cx), ret)] fn collect_nonexhaustive_missing_variants<'a, 'p, 'tcx>( cx: MatchCtxt<'a, 'p, 'tcx>, - column: &PatternColumn<'p, 'tcx>, + column: &PatternColumn<'p, RustcMatchCheckCtxt<'p, 'tcx>>, ) -> Result>, 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); @@ -143,8 +140,8 @@ fn collect_nonexhaustive_missing_variants<'a, 'p, 'tcx>( pub(crate) fn lint_nonexhaustive_missing_variants<'a, 'p, 'tcx>( cx: MatchCtxt<'a, 'p, 'tcx>, - arms: &[MatchArm<'p, 'tcx>], - pat_column: &PatternColumn<'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; diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 223d6cefc83fe..4fb76a891b534 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -31,10 +31,6 @@ pub type DeconstructedPat<'p, 'tcx> = crate::pat::DeconstructedPat<'p, RustcMatchCheckCtxt<'p, 'tcx>>; pub type MatchArm<'p, 'tcx> = crate::MatchArm<'p, RustcMatchCheckCtxt<'p, 'tcx>>; pub type MatchCtxt<'a, 'p, 'tcx> = crate::MatchCtxt<'a, RustcMatchCheckCtxt<'p, 'tcx>>; -pub(crate) type PlaceCtxt<'a, 'p, 'tcx> = - crate::usefulness::PlaceCtxt<'a, RustcMatchCheckCtxt<'p, 'tcx>>; -pub(crate) type SplitConstructorSet<'p, 'tcx> = - crate::constructor::SplitConstructorSet>; pub type Usefulness<'p, 'tcx> = crate::usefulness::Usefulness<'p, RustcMatchCheckCtxt<'p, 'tcx>>; pub type UsefulnessReport<'p, 'tcx> = crate::usefulness::UsefulnessReport<'p, RustcMatchCheckCtxt<'p, 'tcx>>; From cb0e8c508c98590346f5943f1ebd0e901acf87f9 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 24 Jan 2024 21:16:57 +0100 Subject: [PATCH 2/5] Limit the use of `PlaceCtxt` --- .../rustc_pattern_analysis/src/constructor.rs | 16 ++++----- compiler/rustc_pattern_analysis/src/lib.rs | 2 +- compiler/rustc_pattern_analysis/src/lints.rs | 34 +++++++++---------- compiler/rustc_pattern_analysis/src/pat.rs | 7 ++-- compiler/rustc_pattern_analysis/src/rustc.rs | 1 - .../rustc_pattern_analysis/src/usefulness.rs | 34 ++++++++----------- 6 files changed, 42 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 4996015f86345..df5bf3111adc4 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -163,7 +163,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. @@ -818,8 +817,8 @@ impl Constructor { /// 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`. @@ -827,12 +826,11 @@ impl Constructor { /// 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. @@ -873,7 +871,7 @@ impl Constructor { (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:?}" )), } diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index a53d7a0d8096a..866eb67ea9971 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -183,7 +183,7 @@ pub fn analyze_match<'p, 'tcx>( // `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) diff --git a/compiler/rustc_pattern_analysis/src/lints.rs b/compiler/rustc_pattern_analysis/src/lints.rs index e3d218d71c301..4cbd5e70189f2 100644 --- a/compiler/rustc_pattern_analysis/src/lints.rs +++ b/compiler/rustc_pattern_analysis/src/lints.rs @@ -4,8 +4,7 @@ use rustc_span::ErrorGuaranteed; use crate::constructor::{Constructor, SplitConstructorSet}; use crate::errors::{NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered}; use crate::pat::{DeconstructedPat, PatOrWild}; -use crate::rustc::{MatchCtxt, RevealedTy, RustcMatchCheckCtxt, WitnessPat}; -use crate::usefulness::PlaceCtxt; +use crate::rustc::{RevealedTy, RustcMatchCheckCtxt, WitnessPat}; use crate::{MatchArm, TypeCx}; /// A column of patterns in the matrix, where a column is the intuitive notion of "subpatterns that @@ -50,9 +49,9 @@ impl<'p, Cx: TypeCx> PatternColumn<'p, Cx> { } /// Do constructor splitting on the constructors of the column. - fn analyze_ctors(&self, pcx: &PlaceCtxt<'_, Cx>) -> Result, Cx::Error> { + fn analyze_ctors(&self, cx: &Cx, ty: &Cx::Ty) -> Result, Cx::Error> { let column_ctors = self.patterns.iter().map(|p| p.ctor()); - let ctors_for_ty = &pcx.ctors_for_ty()?; + let ctors_for_ty = cx.ctors_for_ty(ty)?; Ok(ctors_for_ty.split(column_ctors)) } @@ -63,10 +62,11 @@ impl<'p, Cx: TypeCx> PatternColumn<'p, Cx> { /// which may change the lengths. fn specialize( &self, - pcx: &PlaceCtxt<'_, Cx>, + cx: &Cx, + ty: &Cx::Ty, ctor: &Constructor, ) -> Vec> { - let arity = ctor.arity(pcx); + let arity = ctor.arity(cx, ty); if arity == 0 { return Vec::new(); } @@ -77,7 +77,7 @@ impl<'p, Cx: TypeCx> PatternColumn<'p, Cx> { 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())); + self.patterns.iter().filter(|pat| ctor.is_covered_by(cx, pat.ctor())); for pat in relevant_patterns { let specialized = pat.specialize(ctor, arity); for (subpat, column) in specialized.into_iter().zip(&mut specialized_columns) { @@ -92,15 +92,14 @@ impl<'p, Cx: TypeCx> PatternColumn<'p, Cx> { /// in a given column. #[instrument(level = "debug", skip(cx), ret)] fn collect_nonexhaustive_missing_variants<'a, 'p, 'tcx>( - cx: MatchCtxt<'a, 'p, 'tcx>, + cx: &RustcMatchCheckCtxt<'p, 'tcx>, column: &PatternColumn<'p, RustcMatchCheckCtxt<'p, 'tcx>>, ) -> Result>, ErrorGuaranteed> { 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), @@ -109,20 +108,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)?; @@ -138,18 +137,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>, +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. diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index d476766d466f2..c94d8b9353589 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -6,7 +6,6 @@ use std::fmt; use smallvec::{smallvec, SmallVec}; use crate::constructor::{Constructor, Slice, SliceKind}; -use crate::usefulness::PlaceCtxt; use crate::{Captures, TypeCx}; use self::Constructor::*; @@ -331,9 +330,9 @@ impl WitnessPat { /// Construct a pattern that matches everything that starts with this constructor. /// For example, if `ctor` is a `Constructor::Variant` for `Option::Some`, we get the pattern /// `Some(_)`. - pub(crate) fn wild_from_ctor(pcx: &PlaceCtxt<'_, Cx>, ctor: Constructor) -> Self { - let fields = pcx.ctor_sub_tys(&ctor).map(|ty| Self::wildcard(ty)).collect(); - Self::new(ctor, fields, pcx.ty.clone()) + pub(crate) fn wild_from_ctor(cx: &Cx, ctor: Constructor, ty: Cx::Ty) -> Self { + let fields = cx.ctor_sub_tys(&ctor, &ty).map(|ty| Self::wildcard(ty)).collect(); + Self::new(ctor, fields, ty) } pub fn ctor(&self) -> &Constructor { diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 4fb76a891b534..ec37e20211860 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -30,7 +30,6 @@ pub type ConstructorSet<'p, 'tcx> = pub type DeconstructedPat<'p, 'tcx> = crate::pat::DeconstructedPat<'p, RustcMatchCheckCtxt<'p, 'tcx>>; pub type MatchArm<'p, 'tcx> = crate::MatchArm<'p, RustcMatchCheckCtxt<'p, 'tcx>>; -pub type MatchCtxt<'a, 'p, 'tcx> = crate::MatchCtxt<'a, RustcMatchCheckCtxt<'p, 'tcx>>; pub type Usefulness<'p, 'tcx> = crate::usefulness::Usefulness<'p, RustcMatchCheckCtxt<'p, 'tcx>>; pub type UsefulnessReport<'p, 'tcx> = crate::usefulness::UsefulnessReport<'p, RustcMatchCheckCtxt<'p, 'tcx>>; diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index b15de1c0ca9d7..62d17ca450982 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -731,20 +731,19 @@ pub fn ensure_sufficient_stack(f: impl FnOnce() -> R) -> R { } /// Context that provides information local to a place under investigation. -pub(crate) struct PlaceCtxt<'a, Cx: TypeCx> { - pub(crate) mcx: MatchCtxt<'a, Cx>, +struct PlaceCtxt<'a, Cx: TypeCx> { + mcx: MatchCtxt<'a, Cx>, /// Type of the place under investigation. - pub(crate) ty: &'a Cx::Ty, + ty: &'a Cx::Ty, } +impl<'a, Cx: TypeCx> Copy for PlaceCtxt<'a, Cx> {} impl<'a, Cx: TypeCx> Clone for PlaceCtxt<'a, Cx> { fn clone(&self) -> Self { Self { mcx: self.mcx, ty: self.ty } } } -impl<'a, Cx: TypeCx> Copy for PlaceCtxt<'a, Cx> {} - impl<'a, Cx: TypeCx> fmt::Debug for PlaceCtxt<'a, Cx> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_struct("PlaceCtxt").field("ty", self.ty).finish() @@ -752,24 +751,21 @@ impl<'a, Cx: TypeCx> fmt::Debug for PlaceCtxt<'a, Cx> { } impl<'a, Cx: TypeCx> PlaceCtxt<'a, Cx> { - /// A `PlaceCtxt` when code other than `is_useful` needs one. - #[cfg_attr(not(feature = "rustc"), allow(dead_code))] - pub(crate) fn new_dummy(mcx: MatchCtxt<'a, Cx>, ty: &'a Cx::Ty) -> Self { - PlaceCtxt { mcx, ty } - } - - pub(crate) fn ctor_arity(&self, ctor: &Constructor) -> usize { + fn ctor_arity(&self, ctor: &Constructor) -> usize { self.mcx.tycx.ctor_arity(ctor, self.ty) } - pub(crate) fn ctor_sub_tys( + fn ctor_sub_tys( &'a self, ctor: &'a Constructor, ) -> impl Iterator + ExactSizeIterator + Captures<'a> { self.mcx.tycx.ctor_sub_tys(ctor, self.ty) } - pub(crate) fn ctors_for_ty(&self) -> Result, Cx::Error> { + fn ctors_for_ty(&self) -> Result, Cx::Error> { self.mcx.tycx.ctors_for_ty(self.ty) } + fn wild_from_ctor(&self, ctor: Constructor) -> WitnessPat { + WitnessPat::wild_from_ctor(self.mcx.tycx, ctor, self.ty.clone()) + } } /// Serves two purposes: @@ -1089,7 +1085,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { wildcard_row_is_relevant: self.wildcard_row_is_relevant && ctor_is_relevant, }; for (i, row) in self.rows().enumerate() { - if ctor.is_covered_by(pcx, row.head().ctor()) { + if ctor.is_covered_by(pcx.mcx.tycx, row.head().ctor()) { let new_row = row.pop_head_constructor(ctor, arity, ctor_is_relevant, i); matrix.expand_and_push(new_row); } @@ -1240,7 +1236,7 @@ impl WitnessStack { /// ``` fn apply_constructor(&mut self, pcx: &PlaceCtxt<'_, Cx>, ctor: &Constructor) { let len = self.0.len(); - let arity = ctor.arity(pcx); + let arity = pcx.ctor_arity(ctor); let fields = self.0.drain((len - arity)..).rev().collect(); let pat = WitnessPat::new(ctor.clone(), fields, pcx.ty.clone()); self.0.push(pat); @@ -1315,20 +1311,20 @@ impl WitnessMatrix { *self = Self::empty(); } else if !report_individual_missing_ctors { // Report `_` as missing. - let pat = WitnessPat::wild_from_ctor(pcx, Constructor::Wildcard); + let pat = pcx.wild_from_ctor(Constructor::Wildcard); self.push_pattern(pat); } else if missing_ctors.iter().any(|c| c.is_non_exhaustive()) { // We need to report a `_` anyway, so listing other constructors would be redundant. // `NonExhaustive` is displayed as `_` just like `Wildcard`, but it will be picked // up by diagnostics to add a note about why `_` is required here. - let pat = WitnessPat::wild_from_ctor(pcx, Constructor::NonExhaustive); + let pat = pcx.wild_from_ctor(Constructor::NonExhaustive); self.push_pattern(pat); } else { // For each missing constructor `c`, we add a `c(_, _, _)` witness appropriately // filled with wildcards. let mut ret = Self::empty(); for ctor in missing_ctors { - let pat = WitnessPat::wild_from_ctor(pcx, ctor.clone()); + let pat = pcx.wild_from_ctor(ctor.clone()); // Clone `self` and add `c(_, _, _)` to each of its witnesses. let mut wit_matrix = self.clone(); wit_matrix.push_pattern(pat); From 83e88c6dfcd7aecf1c49d73e2d9e761d15302341 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 24 Jan 2024 20:57:11 +0100 Subject: [PATCH 3/5] Repurpose `MatchCtxt` for usefulness only --- compiler/rustc_pattern_analysis/src/lib.rs | 18 +------------ .../rustc_pattern_analysis/src/usefulness.rs | 26 ++++++++++++++----- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 866eb67ea9971..99c32de74828a 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -135,20 +135,6 @@ pub trait TypeCx: Sized + fmt::Debug { } } -/// Context that provides information global to a match. -pub struct MatchCtxt<'a, Cx: TypeCx> { - /// The context for type information. - pub tycx: &'a Cx, -} - -impl<'a, Cx: TypeCx> Clone for MatchCtxt<'a, Cx> { - fn clone(&self) -> Self { - Self { tycx: self.tycx } - } -} - -impl<'a, Cx: TypeCx> Copy for MatchCtxt<'a, Cx> {} - /// The arm of a match expression. #[derive(Debug)] pub struct MatchArm<'p, Cx: TypeCx> { @@ -175,9 +161,7 @@ pub fn analyze_match<'p, 'tcx>( ) -> Result, 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. diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 62d17ca450982..d2cf2f72a7d9b 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -719,7 +719,7 @@ use std::fmt; use crate::constructor::{Constructor, ConstructorSet, IntRange}; use crate::pat::{DeconstructedPat, PatOrWild, WitnessPat}; -use crate::{Captures, MatchArm, MatchCtxt, TypeCx}; +use crate::{Captures, MatchArm, TypeCx}; use self::ValidityConstraint::*; @@ -730,9 +730,22 @@ pub fn ensure_sufficient_stack(f: impl FnOnce() -> R) -> R { f() } +/// Context that provides information for usefulness checking. +pub 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 } + } +} + /// Context that provides information local to a place under investigation. struct PlaceCtxt<'a, Cx: TypeCx> { - mcx: MatchCtxt<'a, Cx>, + mcx: UsefulnessCtxt<'a, Cx>, /// Type of the place under investigation. ty: &'a Cx::Ty, } @@ -1358,7 +1371,7 @@ impl WitnessMatrix { /// 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: MatchCtxt<'_, Cx>, + mcx: UsefulnessCtxt<'_, Cx>, overlap_range: IntRange, matrix: &Matrix<'p, Cx>, specialized_matrix: &Matrix<'p, Cx>, @@ -1431,7 +1444,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: MatchCtxt<'a, Cx>, + mcx: UsefulnessCtxt<'a, Cx>, matrix: &mut Matrix<'p, Cx>, is_top_level: bool, ) -> Result, Cx::Error> { @@ -1588,13 +1601,14 @@ pub struct UsefulnessReport<'p, Cx: TypeCx> { } /// Computes whether a match is exhaustive and which of its arms are useful. -#[instrument(skip(cx, arms), level = "debug")] +#[instrument(skip(tycx, arms), level = "debug")] pub fn compute_match_usefulness<'p, Cx: TypeCx>( - cx: MatchCtxt<'_, Cx>, + tycx: &Cx, arms: &[MatchArm<'p, Cx>], scrut_ty: Cx::Ty, scrut_validity: ValidityConstraint, ) -> Result, Cx::Error> { + let cx = UsefulnessCtxt { tycx }; let mut matrix = Matrix::new(arms, scrut_ty, scrut_validity); let non_exhaustiveness_witnesses = compute_exhaustiveness_and_usefulness(cx, &mut matrix, true)?; From 6ef836246bc48387bfdbe2bde951037c71350c00 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 24 Jan 2024 21:29:58 +0100 Subject: [PATCH 4/5] Make `PatternColumn` part of the public API --- .../rustc_pattern_analysis/src/constructor.rs | 10 +-- compiler/rustc_pattern_analysis/src/lib.rs | 4 +- compiler/rustc_pattern_analysis/src/lints.rs | 87 +----------------- .../rustc_pattern_analysis/src/pat_column.rs | 90 +++++++++++++++++++ 4 files changed, 101 insertions(+), 90 deletions(-) create mode 100644 compiler/rustc_pattern_analysis/src/pat_column.rs diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index df5bf3111adc4..4be564b1d7b1e 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -948,10 +948,10 @@ pub enum ConstructorSet { /// 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 { - pub(crate) present: SmallVec<[Constructor; 1]>, - pub(crate) missing: Vec>, - pub(crate) missing_empty: Vec>, +pub struct SplitConstructorSet { + pub present: SmallVec<[Constructor; 1]>, + pub missing: Vec>, + pub missing_empty: Vec>, } impl ConstructorSet { @@ -960,7 +960,7 @@ impl ConstructorSet { /// 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> + Clone, ) -> SplitConstructorSet diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 99c32de74828a..3d0eb117d174f 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -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; @@ -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")] diff --git a/compiler/rustc_pattern_analysis/src/lints.rs b/compiler/rustc_pattern_analysis/src/lints.rs index 4cbd5e70189f2..3f1497540d25e 100644 --- a/compiler/rustc_pattern_analysis/src/lints.rs +++ b/compiler/rustc_pattern_analysis/src/lints.rs @@ -1,92 +1,11 @@ use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS; use rustc_span::ErrorGuaranteed; -use crate::constructor::{Constructor, SplitConstructorSet}; +use crate::constructor::Constructor; use crate::errors::{NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered}; -use crate::pat::{DeconstructedPat, PatOrWild}; +use crate::pat_column::PatternColumn; use crate::rustc::{RevealedTy, RustcMatchCheckCtxt, WitnessPat}; -use crate::{MatchArm, TypeCx}; - -/// 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, Cx: TypeCx> { - patterns: Vec<&'p DeconstructedPat<'p, Cx>>, -} - -impl<'p, Cx: TypeCx> PatternColumn<'p, Cx> { - pub(crate) fn new(arms: &[MatchArm<'p, Cx>]) -> 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, Cx>) { - // 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<&Cx::Ty> { - self.patterns.first().map(|pat| pat.ty()) - } - - /// Do constructor splitting on the constructors of the column. - fn analyze_ctors(&self, cx: &Cx, ty: &Cx::Ty) -> Result, Cx::Error> { - let column_ctors = self.patterns.iter().map(|p| p.ctor()); - let ctors_for_ty = cx.ctors_for_ty(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, - cx: &Cx, - ty: &Cx::Ty, - ctor: &Constructor, - ) -> Vec> { - let arity = ctor.arity(cx, ty); - 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(cx, 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::MatchArm; /// Traverse the patterns to collect any variants of a non_exhaustive enum that fail to be mentioned /// in a given column. diff --git a/compiler/rustc_pattern_analysis/src/pat_column.rs b/compiler/rustc_pattern_analysis/src/pat_column.rs new file mode 100644 index 0000000000000..3cacfc491b981 --- /dev/null +++ b/compiler/rustc_pattern_analysis/src/pat_column.rs @@ -0,0 +1,90 @@ +use crate::constructor::{Constructor, SplitConstructorSet}; +use crate::pat::{DeconstructedPat, PatOrWild}; +use crate::{Captures, MatchArm, TypeCx}; + +/// A column of patterns in a match, 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 is not used in the usefulness algorithm; only in lints. +#[derive(Debug)] +pub struct PatternColumn<'p, Cx: TypeCx> { + /// This must not contain an or-pattern. `expand_and_push` takes care to expand them. + patterns: Vec<&'p DeconstructedPat<'p, Cx>>, +} + +impl<'p, Cx: TypeCx> PatternColumn<'p, Cx> { + pub fn new(arms: &[MatchArm<'p, Cx>]) -> 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, Cx>) { + // 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) + } + } + + pub fn head_ty(&self) -> Option<&Cx::Ty> { + self.patterns.first().map(|pat| pat.ty()) + } + pub fn iter<'a>(&'a self) -> impl Iterator> + Captures<'a> { + self.patterns.iter().copied() + } + + /// Do constructor splitting on the constructors of the column. + pub fn analyze_ctors( + &self, + cx: &Cx, + ty: &Cx::Ty, + ) -> Result, Cx::Error> { + let column_ctors = self.patterns.iter().map(|p| p.ctor()); + let ctors_for_ty = cx.ctors_for_ty(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. + pub fn specialize( + &self, + cx: &Cx, + ty: &Cx::Ty, + ctor: &Constructor, + ) -> Vec> { + let arity = ctor.arity(cx, ty); + 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(cx, 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 + } +} From 59031429c5ad9f4002e3f860ce2ef31e90368b6c Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 24 Jan 2024 21:51:17 +0100 Subject: [PATCH 5/5] Separate `PlaceCtxt` from `UsefulnessCtxt` --- .../rustc_pattern_analysis/src/usefulness.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index d2cf2f72a7d9b..3d45d032a9934 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -745,7 +745,7 @@ impl<'a, Cx: TypeCx> Clone for UsefulnessCtxt<'a, Cx> { /// Context that provides information local to a place under investigation. struct PlaceCtxt<'a, Cx: TypeCx> { - mcx: UsefulnessCtxt<'a, Cx>, + cx: &'a Cx, /// Type of the place under investigation. ty: &'a Cx::Ty, } @@ -753,7 +753,7 @@ struct PlaceCtxt<'a, Cx: TypeCx> { impl<'a, Cx: TypeCx> Copy for PlaceCtxt<'a, Cx> {} impl<'a, Cx: TypeCx> Clone for PlaceCtxt<'a, Cx> { fn clone(&self) -> Self { - Self { mcx: self.mcx, ty: self.ty } + Self { cx: self.cx, ty: self.ty } } } @@ -765,19 +765,19 @@ impl<'a, Cx: TypeCx> fmt::Debug for PlaceCtxt<'a, Cx> { impl<'a, Cx: TypeCx> PlaceCtxt<'a, Cx> { fn ctor_arity(&self, ctor: &Constructor) -> usize { - self.mcx.tycx.ctor_arity(ctor, self.ty) + self.cx.ctor_arity(ctor, self.ty) } fn ctor_sub_tys( &'a self, ctor: &'a Constructor, ) -> impl Iterator + ExactSizeIterator + Captures<'a> { - self.mcx.tycx.ctor_sub_tys(ctor, self.ty) + self.cx.ctor_sub_tys(ctor, self.ty) } fn ctors_for_ty(&self) -> Result, Cx::Error> { - self.mcx.tycx.ctors_for_ty(self.ty) + self.cx.ctors_for_ty(self.ty) } fn wild_from_ctor(&self, ctor: Constructor) -> WitnessPat { - WitnessPat::wild_from_ctor(self.mcx.tycx, ctor, self.ty.clone()) + WitnessPat::wild_from_ctor(self.cx, ctor, self.ty.clone()) } } @@ -1098,7 +1098,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { wildcard_row_is_relevant: self.wildcard_row_is_relevant && ctor_is_relevant, }; for (i, row) in self.rows().enumerate() { - if ctor.is_covered_by(pcx.mcx.tycx, row.head().ctor()) { + if ctor.is_covered_by(pcx.cx, row.head().ctor()) { let new_row = row.pop_head_constructor(ctor, arity, ctor_is_relevant, i); matrix.expand_and_push(new_row); } @@ -1478,7 +1478,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( }; debug!("ty: {ty:?}"); - let pcx = &PlaceCtxt { mcx, ty: &ty }; + let pcx = &PlaceCtxt { cx: mcx.tycx, ty: &ty }; let ctors_for_ty = pcx.ctors_for_ty()?; // Whether the place/column we are inspecting is known to contain valid data.