From b41d8bde00930d80f07b7aa90cc0a8e6bd423413 Mon Sep 17 00:00:00 2001 From: dianne Date: Thu, 17 Apr 2025 02:33:24 -0700 Subject: let deref patterns participate in usefulness/exhaustiveness This does not yet handle the case of mixed deref patterns with normal constructors; it'll ICE in `Constructor::is_covered_by`. That'll be fixed in a later commit. --- compiler/rustc_pattern_analysis/src/constructor.rs | 22 ++++++++++++++++++++++ compiler/rustc_pattern_analysis/src/rustc.rs | 18 ++++++++++++------ compiler/rustc_pattern_analysis/src/usefulness.rs | 4 +++- 3 files changed, 37 insertions(+), 7 deletions(-) (limited to 'compiler/rustc_pattern_analysis/src') diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 4ce868f014f..f7a4931c111 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -696,6 +696,10 @@ pub enum Constructor { F128Range(IeeeFloat, IeeeFloat, 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. @@ -740,6 +744,7 @@ impl Clone for Constructor { 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, @@ -856,6 +861,10 @@ impl Constructor { } (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, @@ -932,6 +941,7 @@ impl Constructor { 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, "")?, Or => { for pat in fields { @@ -1039,8 +1049,17 @@ impl ConstructorSet { 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), @@ -1048,6 +1067,9 @@ impl ConstructorSet { } 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); diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 7c12f69f14c..050b48d082b 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -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 => &[], @@ -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 @@ -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() { @@ -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), diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 11ebbea07fa..53638f2a57d 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -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 @@ -866,7 +867,8 @@ impl PlaceValidity { /// inside `&` and union fields where validity is reset to `MaybeInvalid`. fn specialize(self, ctor: &Constructor) -> 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 { -- cgit 1.4.1-3-g733a5 From fb261a179d2c210785b6e9005201e262dac801b5 Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 20 Apr 2025 23:57:09 -0700 Subject: error early when mixing deref patterns with normal constructors Without adding proper support for mixed exhaustiveness, mixing deref patterns with normal constructors would either violate `ConstructorSet::split`'s invariant 4 or 7. We'd either be ignoring rows with normal constructors or we'd have problems in unspecialization from non-disjoint constructors. Checking mixed exhaustivenss similarly to how unions are currently checked should work, but the diagnostics for unions are confusing. Since mixing deref patterns with normal constructors is pretty niche (currently it only makes sense for `Cow`), emitting an error lets us avoid committing to supporting mixed exhaustiveness without a good answer for the diagnostics. --- compiler/rustc_pattern_analysis/messages.ftl | 4 ++ compiler/rustc_pattern_analysis/src/errors.rs | 14 +++++- compiler/rustc_pattern_analysis/src/rustc.rs | 53 ++++++++++++++++++++++ .../usefulness/mixed-constructors.rs | 48 ++++++++++++++++++++ .../usefulness/mixed-constructors.stderr | 43 ++++++++++++++++++ 5 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 tests/ui/pattern/deref-patterns/usefulness/mixed-constructors.rs create mode 100644 tests/ui/pattern/deref-patterns/usefulness/mixed-constructors.stderr (limited to 'compiler/rustc_pattern_analysis/src') diff --git a/compiler/rustc_pattern_analysis/messages.ftl b/compiler/rustc_pattern_analysis/messages.ftl index 41a1d958f10..d3a3107f8e8 100644 --- a/compiler/rustc_pattern_analysis/messages.ftl +++ b/compiler/rustc_pattern_analysis/messages.ftl @@ -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 diff --git a/compiler/rustc_pattern_analysis/src/errors.rs b/compiler/rustc_pattern_analysis/src/errors.rs index e60930d6cd2..156ba973767 100644 --- a/compiler/rustc_pattern_analysis/src/errors.rs +++ b/compiler/rustc_pattern_analysis/src/errors.rs @@ -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; @@ -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, + 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, +} diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 050b48d082b..a89d01dcbbe 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -1106,6 +1106,14 @@ pub fn analyze_match<'p, 'tcx>( scrut_ty: Ty<'tcx>, ) -> Result, 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, @@ -1125,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>( + 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, } diff --git a/tests/ui/pattern/deref-patterns/usefulness/mixed-constructors.rs b/tests/ui/pattern/deref-patterns/usefulness/mixed-constructors.rs new file mode 100644 index 00000000000..f567dc07bb5 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/usefulness/mixed-constructors.rs @@ -0,0 +1,48 @@ +//! Test matches with a mix of ADT constructors and deref patterns. Currently, usefulness analysis +//! doesn't support this, so make sure we catch it beforehand. As a consequence, it takes priority +//! over non-exhaustive match and unreachable pattern errors. +#![feature(deref_patterns)] +#![expect(incomplete_features)] +#![deny(unreachable_patterns)] + +use std::borrow::Cow; + +fn main() { + let cow: Cow<'static, bool> = Cow::Borrowed(&false); + + match cow { + true => {} + //~v ERROR mix of deref patterns and normal constructors + false => {} + Cow::Borrowed(_) => {} + } + + match cow { + Cow::Owned(_) => {} + Cow::Borrowed(_) => {} + true => {} + //~^ ERROR mix of deref patterns and normal constructors + } + + match cow { + _ => {} + Cow::Owned(_) => {} + false => {} + //~^ ERROR mix of deref patterns and normal constructors + } + + match (cow, 0) { + (Cow::Owned(_), 0) => {} + (Cow::Borrowed(_), 0) => {} + (true, 0) => {} + //~^ ERROR mix of deref patterns and normal constructors + } + + match (0, cow) { + (0, Cow::Owned(_)) => {} + (0, Cow::Borrowed(_)) => {} + _ => {} + (1, true) => {} + //~^ ERROR mix of deref patterns and normal constructors + } +} diff --git a/tests/ui/pattern/deref-patterns/usefulness/mixed-constructors.stderr b/tests/ui/pattern/deref-patterns/usefulness/mixed-constructors.stderr new file mode 100644 index 00000000000..5ad24164b98 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/usefulness/mixed-constructors.stderr @@ -0,0 +1,43 @@ +error: mix of deref patterns and normal constructors + --> $DIR/mixed-constructors.rs:16:9 + | +LL | false => {} + | ^^^^^ matches on the result of dereferencing `Cow<'_, bool>` +LL | Cow::Borrowed(_) => {} + | ^^^^^^^^^^^^^^^^ matches directly on `Cow<'_, bool>` + +error: mix of deref patterns and normal constructors + --> $DIR/mixed-constructors.rs:22:9 + | +LL | Cow::Borrowed(_) => {} + | ^^^^^^^^^^^^^^^^ matches directly on `Cow<'_, bool>` +LL | true => {} + | ^^^^ matches on the result of dereferencing `Cow<'_, bool>` + +error: mix of deref patterns and normal constructors + --> $DIR/mixed-constructors.rs:29:9 + | +LL | Cow::Owned(_) => {} + | ^^^^^^^^^^^^^ matches directly on `Cow<'_, bool>` +LL | false => {} + | ^^^^^ matches on the result of dereferencing `Cow<'_, bool>` + +error: mix of deref patterns and normal constructors + --> $DIR/mixed-constructors.rs:36:10 + | +LL | (Cow::Borrowed(_), 0) => {} + | ^^^^^^^^^^^^^^^^ matches directly on `Cow<'_, bool>` +LL | (true, 0) => {} + | ^^^^ matches on the result of dereferencing `Cow<'_, bool>` + +error: mix of deref patterns and normal constructors + --> $DIR/mixed-constructors.rs:43:13 + | +LL | (0, Cow::Borrowed(_)) => {} + | ^^^^^^^^^^^^^^^^ matches directly on `Cow<'_, bool>` +LL | _ => {} +LL | (1, true) => {} + | ^^^^ matches on the result of dereferencing `Cow<'_, bool>` + +error: aborting due to 5 previous errors + -- cgit 1.4.1-3-g733a5