From c6aa16c469146082613aebbd138d1614c5254131 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 19 Dec 2023 18:09:31 +0100 Subject: [PATCH 1/3] Use `derivative` for better derive bounds --- Cargo.lock | 1 + compiler/rustc_pattern_analysis/Cargo.toml | 1 + .../rustc_pattern_analysis/src/constructor.rs | 3 ++- compiler/rustc_pattern_analysis/src/lib.rs | 13 ++++++------ compiler/rustc_pattern_analysis/src/pat.rs | 3 ++- .../rustc_pattern_analysis/src/usefulness.rs | 21 ++++++++----------- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 117d8c2610e2d..5d78e29de0e08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4340,6 +4340,7 @@ dependencies = [ name = "rustc_pattern_analysis" version = "0.0.0" dependencies = [ + "derivative", "rustc_apfloat", "rustc_arena", "rustc_data_structures", diff --git a/compiler/rustc_pattern_analysis/Cargo.toml b/compiler/rustc_pattern_analysis/Cargo.toml index 908d00cf10567..2152f9fda007d 100644 --- a/compiler/rustc_pattern_analysis/Cargo.toml +++ b/compiler/rustc_pattern_analysis/Cargo.toml @@ -5,6 +5,7 @@ edition = "2021" [dependencies] # tidy-alphabetical-start +derivative = "2.2.0" rustc_apfloat = "0.2.0" rustc_arena = { path = "../rustc_arena", optional = true } rustc_data_structures = { path = "../rustc_data_structures", optional = true } diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index af0a7497a34f9..b688051ca9ccb 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -642,7 +642,8 @@ impl OpaqueId { /// `specialize_constructor` returns the list of fields corresponding to a pattern, given a /// constructor. `Constructor::apply` reconstructs the pattern from a pair of `Constructor` and /// `Fields`. -#[derive(Clone, Debug, PartialEq)] +#[derive(derivative::Derivative)] +#[derivative(Debug(bound = ""), Clone(bound = ""), PartialEq(bound = ""))] pub enum Constructor { /// Tuples and structs. Struct, diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index f00e6f18617b9..9f758dd826353 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -49,7 +49,7 @@ impl<'a, T: ?Sized> Captures<'a> for T {} /// Context that provides type information about constructors. /// /// Most of the crate is parameterized on a type that implements this trait. -pub trait TypeCx: Sized + Clone + fmt::Debug { +pub trait TypeCx: Sized + fmt::Debug { /// The type of a pattern. type Ty: Copy + Clone + fmt::Debug; // FIXME: remove Copy /// The index of an enum variant. @@ -86,7 +86,8 @@ pub trait TypeCx: Sized + Clone + fmt::Debug { } /// Context that provides information global to a match. -#[derive(Clone)] +#[derive(derivative::Derivative)] +#[derivative(Clone(bound = ""), Copy(bound = ""))] pub struct MatchCtxt<'a, 'p, Cx: TypeCx> { /// The context for type information. pub tycx: &'a Cx, @@ -94,18 +95,16 @@ pub struct MatchCtxt<'a, 'p, Cx: TypeCx> { pub wildcard_arena: &'a TypedArena>, } -impl<'a, 'p, Cx: TypeCx> Copy for MatchCtxt<'a, 'p, Cx> {} - /// The arm of a match expression. -#[derive(Clone, Debug)] +#[derive(Debug)] +#[derive(derivative::Derivative)] +#[derivative(Clone(bound = ""), Copy(bound = ""))] pub struct MatchArm<'p, Cx: TypeCx> { pub pat: &'p DeconstructedPat<'p, Cx>, pub has_guard: bool, pub arm_data: Cx::ArmData, } -impl<'p, Cx: TypeCx> Copy for MatchArm<'p, Cx> {} - /// The entrypoint for this crate. Computes whether a match is exhaustive and which of its arms are /// useful, and runs some lints. #[cfg(feature = "rustc")] diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 0cc8477b7cdc3..90772003a0aa5 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -160,7 +160,8 @@ impl<'p, Cx: TypeCx> fmt::Debug for DeconstructedPat<'p, Cx> { /// Same idea as `DeconstructedPat`, except this is a fictitious pattern built up for diagnostics /// purposes. As such they don't use interning and can be cloned. -#[derive(Debug, Clone)] +#[derive(derivative::Derivative)] +#[derivative(Debug(bound = ""), Clone(bound = ""))] pub struct WitnessPat { ctor: Constructor, pub(crate) fields: Vec>, diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 6b9fbd7300327..b1190d84c8ef6 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -569,8 +569,10 @@ pub fn ensure_sufficient_stack(f: impl FnOnce() -> R) -> R { } /// Context that provides information local to a place under investigation. -#[derive(Clone)] +#[derive(derivative::Derivative)] +#[derivative(Debug(bound = ""), Clone(bound = ""), Copy(bound = ""))] pub(crate) struct PlaceCtxt<'a, 'p, Cx: TypeCx> { + #[derivative(Debug = "ignore")] pub(crate) mcx: MatchCtxt<'a, 'p, Cx>, /// Type of the place under investigation. pub(crate) ty: Cx::Ty, @@ -596,14 +598,6 @@ impl<'a, 'p, Cx: TypeCx> PlaceCtxt<'a, 'p, Cx> { } } -impl<'a, 'p, Cx: TypeCx> Copy for PlaceCtxt<'a, 'p, Cx> {} - -impl<'a, 'p, Cx: TypeCx> fmt::Debug for PlaceCtxt<'a, 'p, Cx> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("PlaceCtxt").field("ty", &self.ty).finish() - } -} - /// Serves two purposes: /// - in a wildcard, tracks whether the wildcard matches only valid values (i.e. is a binding `_a`) /// or also invalid values (i.e. is a true `_` pattern). @@ -670,7 +664,8 @@ impl fmt::Display for ValidityConstraint { // - 'a allocated by us // - 'p coming from the input // - Cx global compilation context -#[derive(Clone)] +#[derive(derivative::Derivative)] +#[derivative(Clone(bound = ""))] struct PatStack<'a, 'p, Cx: TypeCx> { // Rows of len 1 are very common, which is why `SmallVec[_; 2]` works well. pats: SmallVec<[&'a DeconstructedPat<'p, Cx>; 2]>, @@ -1022,7 +1017,8 @@ impl<'a, 'p, Cx: TypeCx> fmt::Debug for Matrix<'a, 'p, Cx> { /// The final `Pair(Some(_), true)` is then the resulting witness. /// /// See the top of the file for more detailed explanations and examples. -#[derive(Debug, Clone)] +#[derive(derivative::Derivative)] +#[derivative(Debug(bound = ""), Clone(bound = ""))] struct WitnessStack(Vec>); impl WitnessStack { @@ -1069,7 +1065,8 @@ impl WitnessStack { /// /// Just as the `Matrix` starts with a single column, by the end of the algorithm, this has a single /// column, which contains the patterns that are missing for the match to be exhaustive. -#[derive(Debug, Clone)] +#[derive(derivative::Derivative)] +#[derivative(Debug(bound = ""), Clone(bound = ""))] struct WitnessMatrix(Vec>); impl WitnessMatrix { From f6af7478ba66fc016b5baabd689883b88316bfc2 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 19 Dec 2023 18:14:34 +0100 Subject: [PATCH 2/3] Mention the relevant tracking issue next to my `bit_set` hack --- compiler/rustc_index/src/bit_set.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 3ea1a52ae2829..d730ef58deba6 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -502,6 +502,7 @@ impl ChunkedBitSet { }; #[cfg(not(feature = "nightly"))] let mut words = { + // FIXME: unconditionally use `Rc::new_zeroed` once it is stable (#63291). let words = mem::MaybeUninit::<[Word; CHUNK_WORDS]>::zeroed(); // SAFETY: `words` can safely be all zeroes. let words = unsafe { words.assume_init() }; @@ -567,6 +568,7 @@ impl ChunkedBitSet { }; #[cfg(not(feature = "nightly"))] let mut words = { + // FIXME: unconditionally use `Rc::new_zeroed` once it is stable (#63291). let words = mem::MaybeUninit::<[Word; CHUNK_WORDS]>::zeroed(); // SAFETY: `words` can safely be all zeroes. let words = unsafe { words.assume_init() }; From 5fccaee59c290b85612d4adb120b3e745d3f16dd Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Fri, 22 Dec 2023 23:47:44 +0100 Subject: [PATCH 3/3] Clarify the situation with dummy patterns and `PatData` Use an explicit `Option` instead of requiring a `Default` bound --- .../src/thir/pattern/check_match.rs | 6 +++--- compiler/rustc_pattern_analysis/src/lib.rs | 5 ++--- compiler/rustc_pattern_analysis/src/lints.rs | 4 ++-- compiler/rustc_pattern_analysis/src/pat.rs | 18 +++++++++++------- compiler/rustc_pattern_analysis/src/rustc.rs | 6 +++--- .../rustc_pattern_analysis/src/usefulness.rs | 3 +-- 6 files changed, 22 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index d67dc59c6185f..fc6e2cc961c18 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -860,7 +860,7 @@ fn report_arm_reachability<'p, 'tcx>( for (arm, is_useful) in report.arm_usefulness.iter() { match is_useful { Usefulness::Redundant => { - report_unreachable_pattern(*arm.pat.data(), arm.arm_data, catchall) + report_unreachable_pattern(*arm.pat.data().unwrap(), arm.arm_data, catchall) } Usefulness::Useful(redundant_subpats) if redundant_subpats.is_empty() => {} // The arm is reachable, but contains redundant subpatterns (from or-patterns). @@ -869,12 +869,12 @@ fn report_arm_reachability<'p, 'tcx>( // Emit lints in the order in which they occur in the file. redundant_subpats.sort_unstable_by_key(|pat| pat.data()); for pat in redundant_subpats { - report_unreachable_pattern(*pat.data(), arm.arm_data, None); + report_unreachable_pattern(*pat.data().unwrap(), arm.arm_data, None); } } } if !arm.has_guard && catchall.is_none() && pat_is_catchall(arm.pat) { - catchall = Some(*arm.pat.data()); + catchall = Some(*arm.pat.data().unwrap()); } } } diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 9f758dd826353..a1c9b15766676 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -58,9 +58,8 @@ pub trait TypeCx: Sized + fmt::Debug { type StrLit: Clone + PartialEq + fmt::Debug; /// Extra data to store in a match arm. type ArmData: Copy + Clone + fmt::Debug; - /// Extra data to store in a pattern. `Default` needed when we create fictitious wildcard - /// patterns during analysis. - type PatData: Clone + Default; + /// Extra data to store in a pattern. + type PatData: Clone; /// FIXME(Nadrieril): `Cx` should only give us revealed types. fn reveal_opaque_ty(&self, ty: Self::Ty) -> Self::Ty; diff --git a/compiler/rustc_pattern_analysis/src/lints.rs b/compiler/rustc_pattern_analysis/src/lints.rs index 450a5cb0a105a..2be6e8e3db346 100644 --- a/compiler/rustc_pattern_analysis/src/lints.rs +++ b/compiler/rustc_pattern_analysis/src/lints.rs @@ -203,7 +203,7 @@ pub(crate) fn lint_nonexhaustive_missing_variants<'a, 'p, 'tcx>( }; use rustc_errors::DecorateLint; - let mut err = rcx.tcx.sess.struct_span_warn(*arm.pat.data(), ""); + let mut err = rcx.tcx.sess.struct_span_warn(*arm.pat.data().unwrap(), ""); err.set_primary_message(decorator.msg()); decorator.decorate_lint(&mut err); err.emit(); @@ -253,8 +253,8 @@ pub(crate) fn lint_overlapping_range_endpoints<'a, 'p, 'tcx>( let mut suffixes: SmallVec<[_; 1]> = Default::default(); // Iterate on patterns that contained `overlap`. for pat in column.iter() { - let this_span = *pat.data(); let Constructor::IntRange(this_range) = pat.ctor() else { continue }; + let this_span = *pat.data().unwrap(); if this_range.is_singleton() { // Don't lint when one of the ranges is a singleton. continue; diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 90772003a0aa5..9efd3e864dac0 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -26,14 +26,16 @@ pub struct DeconstructedPat<'p, Cx: TypeCx> { ctor: Constructor, fields: &'p [DeconstructedPat<'p, Cx>], ty: Cx::Ty, - data: Cx::PatData, + /// 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, /// Whether removing this arm would change the behavior of the match expression. useful: Cell, } impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> { - pub fn wildcard(ty: Cx::Ty, data: Cx::PatData) -> Self { - Self::new(Wildcard, &[], ty, data) + pub fn wildcard(ty: Cx::Ty) -> Self { + DeconstructedPat { ctor: Wildcard, fields: &[], ty, data: None, useful: Cell::new(false) } } pub fn new( @@ -42,7 +44,7 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> { ty: Cx::Ty, data: Cx::PatData, ) -> Self { - DeconstructedPat { ctor, fields, ty, data, useful: Cell::new(false) } + DeconstructedPat { ctor, fields, ty, data: Some(data), useful: Cell::new(false) } } pub(crate) fn is_or_pat(&self) -> bool { @@ -63,8 +65,10 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> { pub fn ty(&self) -> Cx::Ty { self.ty } - pub fn data(&self) -> &Cx::PatData { - &self.data + /// Returns the extra data stored in a pattern. Returns `None` if the pattern is a wildcard that + /// does not correspond to a user-supplied pattern. + pub fn data(&self) -> Option<&Cx::PatData> { + self.data.as_ref() } pub fn iter_fields<'a>( @@ -83,7 +87,7 @@ impl<'p, Cx: TypeCx> DeconstructedPat<'p, Cx> { let wildcard_sub_tys = || { let tys = pcx.ctor_sub_tys(other_ctor); tys.iter() - .map(|ty| DeconstructedPat::wildcard(*ty, Cx::PatData::default())) + .map(|ty| DeconstructedPat::wildcard(*ty)) .map(|pat| pcx.mcx.wildcard_arena.alloc(pat) as &_) .collect() }; diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index d2cfb9a8b068c..adaa2ecbe4fd0 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -416,7 +416,7 @@ impl<'p, 'tcx> RustcMatchCheckCtxt<'p, 'tcx> { ty::Tuple(fs) => { ctor = Struct; let mut wilds: SmallVec<[_; 2]> = - fs.iter().map(|ty| DeconstructedPat::wildcard(ty, pat.span)).collect(); + fs.iter().map(|ty| DeconstructedPat::wildcard(ty)).collect(); for pat in subpatterns { wilds[pat.field.index()] = self.lower_pat(&pat.pattern); } @@ -439,7 +439,7 @@ impl<'p, 'tcx> RustcMatchCheckCtxt<'p, 'tcx> { let pat = if let Some(pat) = pattern { self.lower_pat(&pat.pattern) } else { - DeconstructedPat::wildcard(args.type_at(0), pat.span) + DeconstructedPat::wildcard(args.type_at(0)) }; ctor = Struct; fields = singleton(pat); @@ -464,7 +464,7 @@ impl<'p, 'tcx> RustcMatchCheckCtxt<'p, 'tcx> { ty }); let mut wilds: SmallVec<[_; 2]> = - tys.map(|ty| DeconstructedPat::wildcard(ty, pat.span)).collect(); + tys.map(|ty| DeconstructedPat::wildcard(ty)).collect(); for pat in subpatterns { if let Some(i) = field_id_to_id[pat.field.index()] { wilds[i] = self.lower_pat(&pat.pattern); diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index b1190d84c8ef6..a7cd944c0f1bd 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -840,8 +840,7 @@ impl<'a, 'p, Cx: TypeCx> Matrix<'a, 'p, Cx> { scrut_ty: Cx::Ty, scrut_validity: ValidityConstraint, ) -> Self { - let wild_pattern = - wildcard_arena.alloc(DeconstructedPat::wildcard(scrut_ty, Default::default())); + let wild_pattern = wildcard_arena.alloc(DeconstructedPat::wildcard(scrut_ty)); let wildcard_row = PatStack::from_pattern(wild_pattern); let mut matrix = Matrix { rows: Vec::with_capacity(arms.len()),