From be29cd173a87a7d4a87ec6db62f26c3b7666d14a Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 7 Feb 2024 18:59:40 +0100 Subject: [PATCH] Use a unique id instead of by-address indexing --- compiler/rustc_pattern_analysis/src/pat.rs | 17 ++++++- .../rustc_pattern_analysis/src/usefulness.rs | 44 +++++-------------- 2 files changed, 25 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 04a77faad74f8..d9b2b31643d68 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -9,6 +9,17 @@ 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. A `DeconstructedPat` will almost always come from user input; the only /// exception are some `Wildcard`s introduced during pattern lowering. @@ -24,11 +35,13 @@ pub struct DeconstructedPat { /// 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, + /// Globally-unique id used to track usefulness at the level of subpatterns. + pub(crate) uid: PatId, } impl DeconstructedPat { pub fn wildcard(ty: Cx::Ty) -> Self { - DeconstructedPat { ctor: Wildcard, fields: Vec::new(), ty, data: None } + DeconstructedPat { ctor: Wildcard, fields: Vec::new(), ty, data: None, uid: PatId::new() } } pub fn new( @@ -37,7 +50,7 @@ impl DeconstructedPat { ty: Cx::Ty, data: Cx::PatData, ) -> Self { - DeconstructedPat { ctor, fields, ty, data: Some(data) } + DeconstructedPat { ctor, fields, ty, data: Some(data), uid: PatId::new() } } pub(crate) fn is_or_pat(&self) -> bool { diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index bcc734dc81c0c..0cbf6203ada9a 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -713,10 +713,9 @@ 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}; +use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat}; use crate::{Captures, MatchArm, TypeCx}; use self::ValidityConstraint::*; @@ -728,36 +727,13 @@ pub fn ensure_sufficient_stack(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); - -impl ByAddress { - fn addr(&self) -> *const T::Target { - (&*self.0) as *const _ - } -} -/// Raw pointer hashing and comparison. -impl std::hash::Hash for ByAddress { - fn hash(&self, state: &mut H) { - self.addr().hash(state) - } -} -impl PartialEq for ByAddress { - fn eq(&self, other: &Self) -> bool { - std::ptr::eq(self.addr(), other.addr()) - } -} -impl Eq for ByAddress {} - /// Context that provides information for usefulness checking. -struct UsefulnessCtxt<'a, 'p, Cx: TypeCx> { +struct UsefulnessCtxt<'a, Cx: TypeCx> { /// The context for type information. 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>>, + /// unreachable (sub)patterns. + useful_subpatterns: FxHashSet, } /// Context that provides information local to a place under investigation. @@ -1398,7 +1374,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: &mut UsefulnessCtxt<'_, 'p, Cx>, + mcx: &mut UsefulnessCtxt<'_, Cx>, overlap_range: IntRange, matrix: &Matrix<'p, Cx>, specialized_matrix: &Matrix<'p, Cx>, @@ -1471,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: &mut UsefulnessCtxt<'a, 'p, Cx>, + mcx: &mut UsefulnessCtxt<'a, Cx>, matrix: &mut Matrix<'p, Cx>, ) -> Result, Cx::Error> { debug_assert!(matrix.rows().all(|r| r.len() == matrix.column_count())); @@ -1598,7 +1574,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( for row in matrix.rows() { if row.useful { if let PatOrWild::Pat(pat) = row.head() { - mcx.useful_subpatterns.insert(ByAddress(pat)); + mcx.useful_subpatterns.insert(pat.uid); } } } @@ -1620,14 +1596,14 @@ 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>>, + useful_subpatterns: &FxHashSet, pat: &'p DeconstructedPat, ) -> Usefulness<'p, Cx> { fn pat_is_useful<'p, Cx: TypeCx>( - useful_subpatterns: &FxHashSet>>, + useful_subpatterns: &FxHashSet, pat: &'p DeconstructedPat, ) -> bool { - if useful_subpatterns.contains(&ByAddress(pat)) { + if useful_subpatterns.contains(&pat.uid) { true } else if pat.is_or_pat() && pat.iter_fields().any(|f| pat_is_useful(useful_subpatterns, f)) {