Skip to content

allow deref patterns to participate in exhaustiveness analysis #140106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/rustc_pattern_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ pattern_analysis_excluside_range_missing_max = exclusive range missing `{$max}`
.label = this range doesn't match `{$max}` because `..` is an exclusive range
.suggestion = use an inclusive range instead

pattern_analysis_mixed_deref_pattern_constructors = mix of deref patterns and normal constructors
.deref_pattern_label = matches on the result of dereferencing `{$smart_pointer_ty}`
.normal_constructor_label = matches directly on `{$smart_pointer_ty}`

pattern_analysis_non_exhaustive_omitted_pattern = some variants are not matched explicitly
.help = ensure that all variants are matched explicitly by adding the suggested match arms
.note = the matched value is of type `{$scrut_ty}` and the `non_exhaustive_omitted_patterns` attribute was found
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_pattern_analysis/src/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,10 @@ pub enum Constructor<Cx: PatCx> {
F128Range(IeeeFloat<QuadS>, IeeeFloat<QuadS>, RangeEnd),
/// String literals. Strings are not quite the same as `&[u8]` so we treat them separately.
Str(Cx::StrLit),
/// Deref patterns (enabled by the `deref_patterns` feature) provide a way of matching on a
/// smart pointer ADT through its pointee. They don't directly correspond to ADT constructors,
/// and currently are not supported alongside them. Carries the type of the pointee.
DerefPattern(Cx::Ty),
/// Constants that must not be matched structurally. They are treated as black boxes for the
/// purposes of exhaustiveness: we must not inspect them, and they don't count towards making a
/// match exhaustive.
Expand Down Expand Up @@ -740,6 +744,7 @@ impl<Cx: PatCx> Clone for Constructor<Cx> {
Constructor::F64Range(lo, hi, end) => Constructor::F64Range(*lo, *hi, *end),
Constructor::F128Range(lo, hi, end) => Constructor::F128Range(*lo, *hi, *end),
Constructor::Str(value) => Constructor::Str(value.clone()),
Constructor::DerefPattern(ty) => Constructor::DerefPattern(ty.clone()),
Constructor::Opaque(inner) => Constructor::Opaque(inner.clone()),
Constructor::Or => Constructor::Or,
Constructor::Never => Constructor::Never,
Expand Down Expand Up @@ -856,6 +861,10 @@ impl<Cx: PatCx> Constructor<Cx> {
}
(Slice(self_slice), Slice(other_slice)) => self_slice.is_covered_by(*other_slice),

// Deref patterns only interact with other deref patterns. Prior to usefulness analysis,
// we ensure they don't appear alongside any other non-wild non-opaque constructors.
(DerefPattern(_), DerefPattern(_)) => true,

// Opaque constructors don't interact with anything unless they come from the
// syntactically identical pattern.
(Opaque(self_id), Opaque(other_id)) => self_id == other_id,
Expand Down Expand Up @@ -932,6 +941,7 @@ impl<Cx: PatCx> Constructor<Cx> {
F64Range(lo, hi, end) => write!(f, "{lo}{end}{hi}")?,
F128Range(lo, hi, end) => write!(f, "{lo}{end}{hi}")?,
Str(value) => write!(f, "{value:?}")?,
DerefPattern(_) => write!(f, "deref!({:?})", fields.next().unwrap())?,
Opaque(..) => write!(f, "<constant pattern>")?,
Or => {
for pat in fields {
Expand Down Expand Up @@ -1039,15 +1049,27 @@ impl<Cx: PatCx> ConstructorSet<Cx> {
let mut missing = Vec::new();
// Constructors in `ctors`, except wildcards and opaques.
let mut seen = Vec::new();
// If we see a deref pattern, it must be the only non-wildcard non-opaque constructor; we
// ensure this prior to analysis.
let mut deref_pat_present = false;
for ctor in ctors.cloned() {
match ctor {
DerefPattern(..) => {
if !deref_pat_present {
deref_pat_present = true;
present.push(ctor);
}
}
Opaque(..) => present.push(ctor),
Wildcard => {} // discard wildcards
_ => seen.push(ctor),
}
}

match self {
_ if deref_pat_present => {
// Deref patterns are the only constructor; nothing is missing.
}
ConstructorSet::Struct { empty } => {
if !seen.is_empty() {
present.push(Struct);
Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_pattern_analysis/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rustc_errors::{Diag, EmissionGuarantee, Subdiagnostic};
use rustc_macros::{LintDiagnostic, Subdiagnostic};
use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
use rustc_middle::ty::Ty;
use rustc_span::Span;

Expand Down Expand Up @@ -133,3 +133,15 @@ pub(crate) struct NonExhaustiveOmittedPatternLintOnArm {
pub lint_level: &'static str,
pub lint_name: &'static str,
}

#[derive(Diagnostic)]
#[diag(pattern_analysis_mixed_deref_pattern_constructors)]
pub(crate) struct MixedDerefPatternConstructors<'tcx> {
#[primary_span]
pub spans: Vec<Span>,
pub smart_pointer_ty: Ty<'tcx>,
#[label(pattern_analysis_deref_pattern_label)]
pub deref_pattern_label: Span,
#[label(pattern_analysis_normal_constructor_label)]
pub normal_constructor_label: Span,
}
71 changes: 65 additions & 6 deletions compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
}
_ => bug!("bad slice pattern {:?} {:?}", ctor, ty),
},
DerefPattern(pointee_ty) => reveal_and_alloc(cx, once(pointee_ty.inner())),
Bool(..) | IntRange(..) | F16Range(..) | F32Range(..) | F64Range(..)
| F128Range(..) | Str(..) | Opaque(..) | Never | NonExhaustive | Hidden | Missing
| PrivateUninhabited | Wildcard => &[],
Expand Down Expand Up @@ -296,7 +297,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
}
_ => bug!("Unexpected type for constructor `{ctor:?}`: {ty:?}"),
},
Ref => 1,
Ref | DerefPattern(_) => 1,
Slice(slice) => slice.arity(),
Bool(..) | IntRange(..) | F16Range(..) | F32Range(..) | F64Range(..)
| F128Range(..) | Str(..) | Opaque(..) | Never | NonExhaustive | Hidden | Missing
Expand Down Expand Up @@ -493,11 +494,15 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
),
};
}
PatKind::DerefPattern { .. } => {
// FIXME(deref_patterns): At least detect that `box _` is irrefutable.
fields = vec![];
arity = 0;
ctor = Opaque(OpaqueId::new());
PatKind::DerefPattern { subpattern, .. } => {
// NB(deref_patterns): This assumes the deref pattern is matching on a trusted
// `DerefPure` type. If the `Deref` impl isn't trusted, exhaustiveness must take
// into account that multiple calls to deref may return different results. Hence
// multiple deref! patterns cannot be exhaustive together unless each is exhaustive
// by itself.
fields = vec![self.lower_pat(subpattern).at_index(0)];
arity = 1;
ctor = DerefPattern(cx.reveal_opaque_ty(subpattern.ty));
}
PatKind::Leaf { subpatterns } | PatKind::Variant { subpatterns, .. } => {
match ty.kind() {
Expand Down Expand Up @@ -874,6 +879,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
print::write_ref_like(&mut s, pat.ty().inner(), &print(&pat.fields[0])).unwrap();
s
}
DerefPattern(_) => format!("deref!({})", print(&pat.fields[0])),
Slice(slice) => {
let (prefix_len, has_dot_dot) = match slice.kind {
SliceKind::FixedLen(len) => (len, false),
Expand Down Expand Up @@ -1100,6 +1106,14 @@ pub fn analyze_match<'p, 'tcx>(
scrut_ty: Ty<'tcx>,
) -> Result<UsefulnessReport<'p, 'tcx>, ErrorGuaranteed> {
let scrut_ty = tycx.reveal_opaque_ty(scrut_ty);

// The analysis doesn't support deref patterns mixed with normal constructors; error if present.
// FIXME(deref_patterns): This only needs to run when a deref pattern was found during lowering.
if tycx.tcx.features().deref_patterns() {
let pat_column = PatternColumn::new(arms);
detect_mixed_deref_pat_ctors(tycx, &pat_column)?;
}

let scrut_validity = PlaceValidity::from_bool(tycx.known_valid_scrutinee);
let report = compute_match_usefulness(
tycx,
Expand All @@ -1119,6 +1133,51 @@ pub fn analyze_match<'p, 'tcx>(
Ok(report)
}

// FIXME(deref_patterns): Currently it's the responsibility of the frontend (rustc or rust-analyzer)
// to ensure that deref patterns don't appear in the same column as normal constructors. Deref
// patterns aren't currently implemented in rust-analyzer, but should they be, the columnwise check
// here could be made generic and shared between frontends.
fn detect_mixed_deref_pat_ctors<'p, 'tcx>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better home for this anywhere? The other use of PatternColumn I could find was in lints.rs, but this isn't really a lint, so it didn't feel right there.

Copy link
Contributor Author

@dianne dianne Apr 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's looking like maybe this should work for generic PatCx so if/when r-a supports deref patterns it'll be able to run before match analysis there too. So it definitely shouldn't be in rustc.rs or lints.rs. usefulness.rs doesn't quite feel right for something using PatColumn, though it probably should be usefulness::compute_match_usefulness that calls it. pat_column.rs is only the implementation of PatColumn so I'm not sure that's right either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into rehousing this and making it work for generic PatCx on my local branch. It's a bit awkward, though: in order to avoid doing a pass for every single pattern, it'd still be the responsibility of the frontend to report when the pattern needs to be checked for mixed constructors (i.e. when a deref pattern was lowered), so the API is a bit awkward. Maybe an extra pass for every pattern is inconsequential perf though? I'm not sure what would be best, but I can open a perf experiment if needed. In the mean time, I'll think more about how to handle diagnostics for mixed exhaustiveness, to maybe avoid this check altogether.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, perf experiment seems the only way to know

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for where to house it, we could rename lints.rs to checks.rs and put it there?

cx: &RustcPatCtxt<'p, 'tcx>,
column: &PatternColumn<'p, RustcPatCtxt<'p, 'tcx>>,
) -> Result<(), ErrorGuaranteed> {
let Some(&ty) = column.head_ty() else {
return Ok(());
};

// Check for a mix of deref patterns and normal constructors.
let mut normal_ctor_span = None;
let mut deref_pat_span = None;
for pat in column.iter() {
match pat.ctor() {
// The analysis can handle mixing deref patterns with wildcards and opaque patterns.
Wildcard | Opaque(_) => {}
DerefPattern(_) => deref_pat_span = Some(pat.data().span),
// Nothing else can be compared to deref patterns in `Constructor::is_covered_by`.
_ => normal_ctor_span = Some(pat.data().span),
}
}
if let Some(normal_constructor_label) = normal_ctor_span
&& let Some(deref_pattern_label) = deref_pat_span
{
return Err(cx.tcx.dcx().emit_err(errors::MixedDerefPatternConstructors {
spans: vec![deref_pattern_label, normal_constructor_label],
smart_pointer_ty: ty.inner(),
deref_pattern_label,
normal_constructor_label,
}));
}

// Specialize and recurse into the patterns' fields.
let set = column.analyze_ctors(cx, &ty)?;
for ctor in set.present {
for specialized_column in column.specialize(cx, &ty, &ctor).iter() {
detect_mixed_deref_pat_ctors(cx, specialized_column)?;
}
}
Ok(())
}

struct RecursiveOpaque {
def_id: DefId,
}
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_pattern_analysis/src/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,7 @@
//! - `ui/consts/const_in_pattern`
//! - `ui/rfc-2008-non-exhaustive`
//! - `ui/half-open-range-patterns`
//! - `ui/pattern/deref-patterns`
//! - probably many others
//!
//! I (Nadrieril) prefer to put new tests in `ui/pattern/usefulness` unless there's a specific
Expand Down Expand Up @@ -866,7 +867,8 @@ impl PlaceValidity {
/// inside `&` and union fields where validity is reset to `MaybeInvalid`.
fn specialize<Cx: PatCx>(self, ctor: &Constructor<Cx>) -> Self {
// We preserve validity except when we go inside a reference or a union field.
if matches!(ctor, Constructor::Ref | Constructor::UnionField) {
if matches!(ctor, Constructor::Ref | Constructor::DerefPattern(_) | Constructor::UnionField)
{
// Validity of `x: &T` does not imply validity of `*x: T`.
MaybeInvalid
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ Like [`box_patterns`], deref patterns may move out of boxes:
# #![feature(deref_patterns)]
# #![allow(incomplete_features)]
struct NoCopy;
// Match exhaustiveness analysis is not yet implemented.
let deref!(x) = Box::new(NoCopy) else { unreachable!() };
let deref!(x) = Box::new(NoCopy);
drop::<NoCopy>(x);
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ impl<'db> MatchCheckCtx<'db> {
// ignore this issue.
Ref => PatKind::Deref { subpattern: subpatterns.next().unwrap() },
Slice(_) => unimplemented!(),
DerefPattern(_) => unimplemented!(),
&Str(void) => match void {},
Wildcard | NonExhaustive | Hidden | PrivateUninhabited => PatKind::Wild,
Never => PatKind::Never,
Expand Down Expand Up @@ -351,6 +352,7 @@ impl PatCx for MatchCheckCtx<'_> {
},
Ref => 1,
Slice(..) => unimplemented!(),
DerefPattern(..) => unimplemented!(),
Never | Bool(..) | IntRange(..) | F16Range(..) | F32Range(..) | F64Range(..)
| F128Range(..) | Str(..) | Opaque(..) | NonExhaustive | PrivateUninhabited
| Hidden | Missing | Wildcard => 0,
Expand Down Expand Up @@ -411,6 +413,7 @@ impl PatCx for MatchCheckCtx<'_> {
}
},
Slice(_) => unreachable!("Found a `Slice` constructor in match checking"),
DerefPattern(_) => unreachable!("Found a `DerefPattern` constructor in match checking"),
Never | Bool(..) | IntRange(..) | F16Range(..) | F32Range(..) | F64Range(..)
| F128Range(..) | Str(..) | Opaque(..) | NonExhaustive | PrivateUninhabited
| Hidden | Missing | Wildcard => {
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/pattern/deref-patterns/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ fn simple_vec(vec: Vec<u32>) -> u32 {
deref!([x]) => x,
deref!([1, x]) => x + 200,
deref!(ref slice) => slice.iter().sum(),
_ => 2000,
}
}

Expand All @@ -25,7 +24,6 @@ fn simple_vec(vec: Vec<u32>) -> u32 {
[x] => x,
[1, x] => x + 200,
deref!(ref slice) => slice.iter().sum(),
_ => 2000,
}
}

Expand Down
8 changes: 4 additions & 4 deletions tests/ui/pattern/deref-patterns/closure_capture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ struct NoCopy;
fn main() {
let b = Rc::new("aaa".to_string());
let f = || {
let deref!(ref s) = b else { unreachable!() };
let deref!(ref s) = b;
assert_eq!(s.len(), 3);
};
assert_eq!(b.len(), 3);
Expand All @@ -26,7 +26,7 @@ fn main() {

let mut b = "aaa".to_string();
let mut f = || {
let deref!(ref mut s) = b else { unreachable!() };
let deref!(ref mut s) = b;
s.make_ascii_uppercase();
};
f();
Expand All @@ -53,15 +53,15 @@ fn main() {
let b = Box::new(NoCopy);
let f = || {
// this should move out of the box rather than borrow.
let deref!(x) = b else { unreachable!() };
let deref!(x) = b;
drop::<NoCopy>(x);
};
f();

let b = Box::new((NoCopy,));
let f = || {
// this should move out of the box rather than borrow.
let (x,) = b else { unreachable!() };
let (x,) = b;
drop::<NoCopy>(x);
};
f();
Expand Down
10 changes: 5 additions & 5 deletions tests/ui/pattern/deref-patterns/deref-box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@
#![expect(incomplete_features)]

fn unbox_1<T>(b: Box<T>) -> T {
let deref!(x) = b else { unreachable!() };
let deref!(x) = b;
x
}

fn unbox_2<T>(b: Box<(T,)>) -> T {
let (x,) = b else { unreachable!() };
let (x,) = b;
x
}

fn unbox_separately<T>(b: Box<(T, T)>) -> (T, T) {
let (x, _) = b else { unreachable!() };
let (_, y) = b else { unreachable!() };
let (x, _) = b;
let (_, y) = b;
(x, y)
}

Expand All @@ -31,7 +31,7 @@ fn main() {

// test that borrowing from a box also works
let mut b = "hi".to_owned().into_boxed_str();
let deref!(ref mut s) = b else { unreachable!() };
let deref!(ref mut s) = b;
s.make_ascii_uppercase();
assert_eq!(&*b, "HI");
}
4 changes: 0 additions & 4 deletions tests/ui/pattern/deref-patterns/implicit-cow-deref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ fn main() {

match cow {
[..] => {}
_ => unreachable!(),
}

match cow {
Expand All @@ -22,14 +21,12 @@ fn main() {
match Rc::new(&cow) {
Cow::Borrowed { 0: _ } => {}
Cow::Owned { 0: _ } => unreachable!(),
_ => unreachable!(),
}

let cow_of_cow: Cow<'_, Cow<'static, [u8]>> = Cow::Owned(cow);

match cow_of_cow {
[..] => {}
_ => unreachable!(),
}

// This matches on the outer `Cow` (the owned one).
Expand All @@ -41,6 +38,5 @@ fn main() {
match Rc::new(&cow_of_cow) {
Cow::Borrowed { 0: _ } => unreachable!(),
Cow::Owned { 0: _ } => {}
_ => unreachable!(),
}
}
Loading
Loading