Skip to content

Commit 50061f3

Browse files
committed
always check for mixed deref pattern and normal constructors
This makes it work for box patterns and in rust-analyzer.
1 parent d98a5da commit 50061f3

File tree

8 files changed

+117
-53
lines changed

8 files changed

+117
-53
lines changed
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
//! Contains checks that must be run to validate matches before performing usefulness analysis.
2+
3+
use crate::constructor::Constructor::*;
4+
use crate::pat_column::PatternColumn;
5+
use crate::{MatchArm, PatCx};
6+
7+
/// Validate that deref patterns and normal constructors aren't used to match on the same place.
8+
pub(crate) fn detect_mixed_deref_pat_ctors<'p, Cx: PatCx>(
9+
cx: &Cx,
10+
arms: &[MatchArm<'p, Cx>],
11+
) -> Result<(), Cx::Error> {
12+
let pat_column = PatternColumn::new(arms);
13+
detect_mixed_deref_pat_ctors_inner(cx, &pat_column)
14+
}
15+
16+
fn detect_mixed_deref_pat_ctors_inner<'p, Cx: PatCx>(
17+
cx: &Cx,
18+
column: &PatternColumn<'p, Cx>,
19+
) -> Result<(), Cx::Error> {
20+
let Some(ty) = column.head_ty() else {
21+
return Ok(());
22+
};
23+
24+
// Check for a mix of deref patterns and normal constructors.
25+
let mut deref_pat = None;
26+
let mut normal_pat = None;
27+
for pat in column.iter() {
28+
match pat.ctor() {
29+
// The analysis can handle mixing deref patterns with wildcards and opaque patterns.
30+
Wildcard | Opaque(_) => {}
31+
DerefPattern(_) => deref_pat = Some(pat),
32+
// Nothing else can be compared to deref patterns in `Constructor::is_covered_by`.
33+
_ => normal_pat = Some(pat),
34+
}
35+
}
36+
if let Some(deref_pat) = deref_pat
37+
&& let Some(normal_pat) = normal_pat
38+
{
39+
return Err(cx.report_mixed_deref_pat_ctors(deref_pat, normal_pat));
40+
}
41+
42+
// Specialize and recurse into the patterns' fields.
43+
let set = column.analyze_ctors(cx, &ty)?;
44+
for ctor in set.present {
45+
for specialized_column in column.specialize(cx, &ty, &ctor).iter() {
46+
detect_mixed_deref_pat_ctors_inner(cx, specialized_column)?;
47+
}
48+
}
49+
Ok(())
50+
}

compiler/rustc_pattern_analysis/src/lib.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#![allow(unused_crate_dependencies)]
99
// tidy-alphabetical-end
1010

11+
pub(crate) mod checks;
1112
pub mod constructor;
1213
#[cfg(feature = "rustc")]
1314
pub mod errors;
@@ -107,6 +108,15 @@ pub trait PatCx: Sized + fmt::Debug {
107108
_gapped_with: &[&DeconstructedPat<Self>],
108109
) {
109110
}
111+
112+
/// The current implementation of deref patterns requires that they can't match on the same
113+
/// place as a normal constructor. Since this isn't caught by type-checking, we check it in the
114+
/// `PatCx` before running the analysis. This reports an error if the check fails.
115+
fn report_mixed_deref_pat_ctors(
116+
&self,
117+
deref_pat: &DeconstructedPat<Self>,
118+
normal_pat: &DeconstructedPat<Self>,
119+
) -> Self::Error;
110120
}
111121

112122
/// The arm of a match expression.

compiler/rustc_pattern_analysis/src/rustc.rs

Lines changed: 15 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,6 +1027,21 @@ impl<'p, 'tcx: 'p> PatCx for RustcPatCtxt<'p, 'tcx> {
10271027
);
10281028
}
10291029
}
1030+
1031+
fn report_mixed_deref_pat_ctors(
1032+
&self,
1033+
deref_pat: &crate::pat::DeconstructedPat<Self>,
1034+
normal_pat: &crate::pat::DeconstructedPat<Self>,
1035+
) -> Self::Error {
1036+
let deref_pattern_label = deref_pat.data().span;
1037+
let normal_constructor_label = normal_pat.data().span;
1038+
self.tcx.dcx().emit_err(errors::MixedDerefPatternConstructors {
1039+
spans: vec![deref_pattern_label, normal_constructor_label],
1040+
smart_pointer_ty: deref_pat.ty().inner(),
1041+
deref_pattern_label,
1042+
normal_constructor_label,
1043+
})
1044+
}
10301045
}
10311046

10321047
/// Recursively expand this pattern into its subpatterns. Only useful for or-patterns.
@@ -1055,13 +1070,6 @@ pub fn analyze_match<'p, 'tcx>(
10551070
) -> Result<UsefulnessReport<'p, 'tcx>, ErrorGuaranteed> {
10561071
let scrut_ty = tycx.reveal_opaque_ty(scrut_ty);
10571072

1058-
// The analysis doesn't support deref patterns mixed with normal constructors; error if present.
1059-
// FIXME(deref_patterns): This only needs to run when a deref pattern was found during lowering.
1060-
if tycx.tcx.features().deref_patterns() {
1061-
let pat_column = PatternColumn::new(arms);
1062-
detect_mixed_deref_pat_ctors(tycx, &pat_column)?;
1063-
}
1064-
10651073
let scrut_validity = PlaceValidity::from_bool(tycx.known_valid_scrutinee);
10661074
let report = compute_match_usefulness(
10671075
tycx,
@@ -1080,48 +1088,3 @@ pub fn analyze_match<'p, 'tcx>(
10801088

10811089
Ok(report)
10821090
}
1083-
1084-
// FIXME(deref_patterns): Currently it's the responsibility of the frontend (rustc or rust-analyzer)
1085-
// to ensure that deref patterns don't appear in the same column as normal constructors. Deref
1086-
// patterns aren't currently implemented in rust-analyzer, but should they be, the columnwise check
1087-
// here could be made generic and shared between frontends.
1088-
fn detect_mixed_deref_pat_ctors<'p, 'tcx>(
1089-
cx: &RustcPatCtxt<'p, 'tcx>,
1090-
column: &PatternColumn<'p, RustcPatCtxt<'p, 'tcx>>,
1091-
) -> Result<(), ErrorGuaranteed> {
1092-
let Some(&ty) = column.head_ty() else {
1093-
return Ok(());
1094-
};
1095-
1096-
// Check for a mix of deref patterns and normal constructors.
1097-
let mut normal_ctor_span = None;
1098-
let mut deref_pat_span = None;
1099-
for pat in column.iter() {
1100-
match pat.ctor() {
1101-
// The analysis can handle mixing deref patterns with wildcards and opaque patterns.
1102-
Wildcard | Opaque(_) => {}
1103-
DerefPattern(_) => deref_pat_span = Some(pat.data().span),
1104-
// Nothing else can be compared to deref patterns in `Constructor::is_covered_by`.
1105-
_ => normal_ctor_span = Some(pat.data().span),
1106-
}
1107-
}
1108-
if let Some(normal_constructor_label) = normal_ctor_span
1109-
&& let Some(deref_pattern_label) = deref_pat_span
1110-
{
1111-
return Err(cx.tcx.dcx().emit_err(errors::MixedDerefPatternConstructors {
1112-
spans: vec![deref_pattern_label, normal_constructor_label],
1113-
smart_pointer_ty: ty.inner(),
1114-
deref_pattern_label,
1115-
normal_constructor_label,
1116-
}));
1117-
}
1118-
1119-
// Specialize and recurse into the patterns' fields.
1120-
let set = column.analyze_ctors(cx, &ty)?;
1121-
for ctor in set.present {
1122-
for specialized_column in column.specialize(cx, &ty, &ctor).iter() {
1123-
detect_mixed_deref_pat_ctors(cx, specialized_column)?;
1124-
}
1125-
}
1126-
Ok(())
1127-
}

compiler/rustc_pattern_analysis/src/usefulness.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ use tracing::{debug, instrument};
720720
use self::PlaceValidity::*;
721721
use crate::constructor::{Constructor, ConstructorSet, IntRange};
722722
use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat};
723-
use crate::{MatchArm, PatCx, PrivateUninhabitedField};
723+
use crate::{MatchArm, PatCx, PrivateUninhabitedField, checks};
724724
#[cfg(not(feature = "rustc"))]
725725
pub fn ensure_sufficient_stack<R>(f: impl FnOnce() -> R) -> R {
726726
f()
@@ -1836,6 +1836,9 @@ pub fn compute_match_usefulness<'p, Cx: PatCx>(
18361836
scrut_validity: PlaceValidity,
18371837
complexity_limit: usize,
18381838
) -> Result<UsefulnessReport<'p, Cx>, Cx::Error> {
1839+
// The analysis doesn't support deref patterns mixed with normal constructors; error if present.
1840+
checks::detect_mixed_deref_pat_ctors(tycx, arms)?;
1841+
18391842
let mut cx = UsefulnessCtxt {
18401843
tycx,
18411844
branch_usefulness: FxHashMap::default(),

compiler/rustc_pattern_analysis/tests/common/mod.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use rustc_pattern_analysis::constructor::{
22
Constructor, ConstructorSet, IntRange, MaybeInfiniteInt, RangeEnd, VariantVisibility,
33
};
4+
use rustc_pattern_analysis::pat::DeconstructedPat;
45
use rustc_pattern_analysis::usefulness::{PlaceValidity, UsefulnessReport};
56
use rustc_pattern_analysis::{MatchArm, PatCx, PrivateUninhabitedField};
67

@@ -184,6 +185,14 @@ impl PatCx for Cx {
184185
fn complexity_exceeded(&self) -> Result<(), Self::Error> {
185186
Err(())
186187
}
188+
189+
fn report_mixed_deref_pat_ctors(
190+
&self,
191+
_deref_pat: &DeconstructedPat<Self>,
192+
_normal_pat: &DeconstructedPat<Self>,
193+
) -> Self::Error {
194+
panic!("`rustc_pattern_analysis::tests` currently doesn't test deref pattern errors")
195+
}
187196
}
188197

189198
/// Construct a single pattern; see `pats!()`.

src/tools/rust-analyzer/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,14 @@ impl PatCx for MatchCheckCtx<'_> {
527527
fn complexity_exceeded(&self) -> Result<(), Self::Error> {
528528
Err(())
529529
}
530+
531+
fn report_mixed_deref_pat_ctors(
532+
&self,
533+
_deref_pat: &DeconstructedPat<'_>,
534+
_normal_pat: &DeconstructedPat<'_>,
535+
) {
536+
// FIXME(deref_patterns): This could report an error comparable to the one in rustc.
537+
}
530538
}
531539

532540
impl fmt::Debug for MatchCheckCtx<'_> {
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//! Test that `box _` patterns and `Box { .. }` patterns can't be used to match on the same place.
2+
//! This is required for the current implementation of exhaustiveness analysis for deref patterns.
3+
4+
#![feature(box_patterns)]
5+
6+
fn main() {
7+
match Box::new(0) {
8+
box _ => {} //~ ERROR mix of deref patterns and normal constructors
9+
Box { .. } => {}
10+
}
11+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: mix of deref patterns and normal constructors
2+
--> $DIR/box-pattern-constructor-mismatch.rs:8:9
3+
|
4+
LL | box _ => {}
5+
| ^^^^^ matches on the result of dereferencing `Box<i32>`
6+
LL | Box { .. } => {}
7+
| ^^^^^^^^^^ matches directly on `Box<i32>`
8+
9+
error: aborting due to 1 previous error
10+

0 commit comments

Comments
 (0)