From cb3ce6645f09929d2e9419aa0a45134db87ab991 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 24 Jan 2024 23:08:36 +0100 Subject: [PATCH 1/4] Move usefulness-specific pattern computations to `usefulness` --- compiler/rustc_pattern_analysis/src/pat.rs | 37 +++++-------------- .../rustc_pattern_analysis/src/usefulness.rs | 37 ++++++++++++++++--- 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 9bde23c7bf124..2314da7149b00 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -29,7 +29,7 @@ pub struct DeconstructedPat { /// correspond to a user-supplied pattern. data: Option, /// Whether removing this arm would change the behavior of the match expression. - useful: Cell, + pub(crate) useful: Cell, } impl DeconstructedPat { @@ -112,34 +112,17 @@ impl DeconstructedPat { 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) } } } diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 576005b2c7f8f..219d774513d6f 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -1599,6 +1599,36 @@ 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>(pat: &'p DeconstructedPat) -> Usefulness<'p, Cx> { + fn pat_is_useful<'p, Cx: TypeCx>(pat: &'p DeconstructedPat) -> bool { + if pat.useful.get() { + true + } else if pat.is_or_pat() && pat.iter_fields().any(|f| pat_is_useful(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(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(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. @@ -1626,12 +1656,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(arm.pat); (arm, usefulness) }) .collect(); From 9715df3f4459a0e0f68eec64c75a5d5e626ed673 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 7 Feb 2024 04:26:54 +0100 Subject: [PATCH 2/4] Track redundant subpatterns without interior mutability --- .../rustc_pattern_analysis/src/usefulness.rs | 76 ++++++++++++++----- 1 file changed, 55 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 219d774513d6f..092e752e97744 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -713,9 +713,11 @@ //! 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 std::ops::Deref; use crate::constructor::{Constructor, ConstructorSet, IntRange}; use crate::pat::{DeconstructedPat, PatOrWild, WitnessPat}; @@ -730,18 +732,37 @@ 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, -} +/// 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<'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 } +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> { + /// 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>>, +} /// Context that provides information local to a place under investigation. struct PlaceCtxt<'a, Cx: TypeCx> { @@ -1381,7 +1402,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: UsefulnessCtxt<'_, Cx>, + mcx: &mut UsefulnessCtxt<'_, 'p, Cx>, overlap_range: IntRange, matrix: &Matrix<'p, Cx>, specialized_matrix: &Matrix<'p, Cx>, @@ -1454,7 +1475,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, 'p, Cx>, matrix: &mut Matrix<'p, Cx>, ) -> Result, Cx::Error> { debug_assert!(matrix.rows().all(|r| r.len() == matrix.column_count())); @@ -1580,7 +1601,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(ByAddress(pat)); + } } } @@ -1600,11 +1623,18 @@ 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>(pat: &'p DeconstructedPat) -> Usefulness<'p, Cx> { - fn pat_is_useful<'p, Cx: TypeCx>(pat: &'p DeconstructedPat) -> bool { - if pat.useful.get() { +fn collect_pattern_usefulness<'p, Cx: TypeCx>( + useful_subpatterns: &FxHashSet>>, + pat: &'p DeconstructedPat, +) -> Usefulness<'p, Cx> { + fn pat_is_useful<'p, Cx: TypeCx>( + useful_subpatterns: &FxHashSet>>, + pat: &'p DeconstructedPat, + ) -> bool { + if useful_subpatterns.contains(&ByAddress(pat)) { true - } else if pat.is_or_pat() && pat.iter_fields().any(|f| pat_is_useful(f)) { + } 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. @@ -1616,7 +1646,7 @@ fn collect_pattern_usefulness<'p, Cx: TypeCx>(pat: &'p DeconstructedPat) -> let mut redundant_subpats = Vec::new(); pat.walk(&mut |p| { - if pat_is_useful(p) { + if pat_is_useful(useful_subpatterns, p) { // The pattern is useful, so we recurse to find redundant subpatterns. true } else { @@ -1626,7 +1656,11 @@ fn collect_pattern_usefulness<'p, Cx: TypeCx>(pat: &'p DeconstructedPat) -> } }); - if pat_is_useful(pat) { Usefulness::Useful(redundant_subpats) } else { Usefulness::Redundant } + 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. @@ -1646,9 +1680,9 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>( scrut_ty: Cx::Ty, scrut_validity: ValidityConstraint, ) -> Result, 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 @@ -1656,7 +1690,7 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>( .copied() .map(|arm| { debug!(?arm); - let usefulness = collect_pattern_usefulness(arm.pat); + let usefulness = collect_pattern_usefulness(&cx.useful_subpatterns, arm.pat); (arm, usefulness) }) .collect(); From 8465c82b64501a70d3f9ae967987473efb4f151c Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 24 Jan 2024 23:23:14 +0100 Subject: [PATCH 3/4] Cleanup comments and dead code --- compiler/rustc_pattern_analysis/src/pat.rs | 32 +++---------------- .../rustc_pattern_analysis/src/usefulness.rs | 14 +++----- 2 files changed, 9 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 2314da7149b00..04a77faad74f8 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -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}; @@ -11,11 +10,8 @@ use crate::TypeCx; use self::Constructor::*; /// 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 +24,11 @@ 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, - /// Whether removing this arm would change the behavior of the match expression. - pub(crate) useful: Cell, } impl DeconstructedPat { 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 } } pub fn new( @@ -49,7 +37,7 @@ impl DeconstructedPat { 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) } } pub(crate) fn is_or_pat(&self) -> bool { @@ -107,12 +95,6 @@ impl DeconstructedPat { } } - /// 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) - } - /// 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. @@ -267,12 +249,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> { diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 092e752e97744..bcc734dc81c0c 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -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. //! //! //! @@ -1462,8 +1458,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. /// From be29cd173a87a7d4a87ec6db62f26c3b7666d14a Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 7 Feb 2024 18:59:40 +0100 Subject: [PATCH 4/4] 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)) {