Auto merge of #143472 - dianne:deref-pat-column-check, r=Nadrieril

`rustc_pattern_analysis`: always check that deref patterns don't match on the same place as normal constructors

In rust-lang/rust#140106, deref pattern validation was tied to the `deref_patterns` feature to temporarily avoid affecting perf. However:
- As of rust-lang/rust#143414, box patterns are represented as deref patterns in `rustc_pattern_analysis`. Since they can be used by enabling `box_patterns` instead of `deref_patterns`, it was possible for them to skip validation, resulting in an ICE. This fixes that and adds a regression test.
- External tooling (e.g. rust-analyzer) will also need to validate matches containing deref patterns, which was not possible. This fixes that by making `compute_match_usefulness` validate deref patterns by default.

In order to avoid doing an extra pass for anything with patterns, the second commit makes `RustcPatCtxt` keep track of whether it encounters a deref pattern, so that it only does the check if so. This is purely for performance. If the perf impact of the first commit is negligible and the complexity cost introduced by the second commit is significant, it may be worth dropping the latter.

r? `@Nadrieril`

10 files changed, 142 insertions, 53 deletions

diff --git a/compiler/rustc_mir_build/src/builder/scope.rs b/compiler/rustc_mir_build/src/builder/scope.rs
index 5a5456aedc2..12a56d7c5ea 100644
--- a/compiler/rustc_mir_build/src/builder/scope.rs
+++ b/compiler/rustc_mir_build/src/builder/scope.rs
@@ -927,6 +927,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             scrut_span: rustc_span::Span::default(),
             refutable: true,
             known_valid_scrutinee: true,
+            internal_state: Default::default(),
         };
 
         let valtree = match self.eval_unevaluated_mir_constant_to_valtree(constant) {
diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
index b7b160c738d..a49bfc1b8f4 100644
--- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
+++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
@@ -406,6 +406,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
             scrut_span,
             refutable,
             known_valid_scrutinee,
+            internal_state: Default::default(),
         }
     }
 
diff --git a/compiler/rustc_pattern_analysis/src/checks.rs b/compiler/rustc_pattern_analysis/src/checks.rs
new file mode 100644
index 00000000000..88ccaa1e0e5
--- /dev/null
+++ b/compiler/rustc_pattern_analysis/src/checks.rs
@@ -0,0 +1,50 @@
+//! Contains checks that must be run to validate matches before performing usefulness analysis.
+
+use crate::constructor::Constructor::*;
+use crate::pat_column::PatternColumn;
+use crate::{MatchArm, PatCx};
+
+/// Validate that deref patterns and normal constructors aren't used to match on the same place.
+pub(crate) fn detect_mixed_deref_pat_ctors<'p, Cx: PatCx>(
+    cx: &Cx,
+    arms: &[MatchArm<'p, Cx>],
+) -> Result<(), Cx::Error> {
+    let pat_column = PatternColumn::new(arms);
+    detect_mixed_deref_pat_ctors_inner(cx, &pat_column)
+}
+
+fn detect_mixed_deref_pat_ctors_inner<'p, Cx: PatCx>(
+    cx: &Cx,
+    column: &PatternColumn<'p, Cx>,
+) -> Result<(), Cx::Error> {
+    let Some(ty) = column.head_ty() else {
+        return Ok(());
+    };
+
+    // Check for a mix of deref patterns and normal constructors.
+    let mut deref_pat = None;
+    let mut normal_pat = 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 = Some(pat),
+            // Nothing else can be compared to deref patterns in `Constructor::is_covered_by`.
+            _ => normal_pat = Some(pat),
+        }
+    }
+    if let Some(deref_pat) = deref_pat
+        && let Some(normal_pat) = normal_pat
+    {
+        return Err(cx.report_mixed_deref_pat_ctors(deref_pat, normal_pat));
+    }
+
+    // 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_inner(cx, specialized_column)?;
+        }
+    }
+    Ok(())
+}
diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs
index 2b85d7b26ce..66df35f9ee4 100644
--- a/compiler/rustc_pattern_analysis/src/lib.rs
+++ b/compiler/rustc_pattern_analysis/src/lib.rs
@@ -8,6 +8,7 @@
 #![allow(unused_crate_dependencies)]
 // tidy-alphabetical-end
 
+pub(crate) mod checks;
 pub mod constructor;
 #[cfg(feature = "rustc")]
 pub mod errors;
@@ -107,6 +108,20 @@ pub trait PatCx: Sized + fmt::Debug {
         _gapped_with: &[&DeconstructedPat<Self>],
     ) {
     }
+
+    /// Check if we may need to perform additional deref-pattern-specific validation.
+    fn match_may_contain_deref_pats(&self) -> bool {
+        true
+    }
+
+    /// The current implementation of deref patterns requires that they can't match on the same
+    /// place as a normal constructor. Since this isn't caught by type-checking, we check it in the
+    /// `PatCx` before running the analysis. This reports an error if the check fails.
+    fn report_mixed_deref_pat_ctors(
+        &self,
+        deref_pat: &DeconstructedPat<Self>,
+        normal_pat: &DeconstructedPat<Self>,
+    ) -> Self::Error;
 }
 
 /// The arm of a match expression.
diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs
index e53cebc59ba..ee72b676b38 100644
--- a/compiler/rustc_pattern_analysis/src/rustc.rs
+++ b/compiler/rustc_pattern_analysis/src/rustc.rs
@@ -1,3 +1,4 @@
+use std::cell::Cell;
 use std::fmt;
 use std::iter::once;
 
@@ -99,6 +100,16 @@ pub struct RustcPatCtxt<'p, 'tcx: 'p> {
     /// Whether the data at the scrutinee is known to be valid. This is false if the scrutinee comes
     /// from a union field, a pointer deref, or a reference deref (pending opsem decisions).
     pub known_valid_scrutinee: bool,
+    pub internal_state: RustcPatCtxtState,
+}
+
+/// Private fields of [`RustcPatCtxt`], separated out to permit record initialization syntax.
+#[derive(Clone, Default)]
+pub struct RustcPatCtxtState {
+    /// Has a deref pattern been lowered? This is initialized to `false` and is updated by
+    /// [`RustcPatCtxt::lower_pat`] in order to avoid performing deref-pattern-specific validation
+    /// for everything containing patterns.
+    has_lowered_deref_pat: Cell<bool>,
 }
 
 impl<'p, 'tcx: 'p> fmt::Debug for RustcPatCtxt<'p, 'tcx> {
@@ -474,6 +485,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
                 fields = vec![self.lower_pat(subpattern).at_index(0)];
                 arity = 1;
                 ctor = DerefPattern(cx.reveal_opaque_ty(subpattern.ty));
+                self.internal_state.has_lowered_deref_pat.set(true);
             }
             PatKind::Leaf { subpatterns } | PatKind::Variant { subpatterns, .. } => {
                 match ty.kind() {
@@ -1027,6 +1039,25 @@ impl<'p, 'tcx: 'p> PatCx for RustcPatCtxt<'p, 'tcx> {
             );
         }
     }
+
+    fn match_may_contain_deref_pats(&self) -> bool {
+        self.internal_state.has_lowered_deref_pat.get()
+    }
+
+    fn report_mixed_deref_pat_ctors(
+        &self,
+        deref_pat: &crate::pat::DeconstructedPat<Self>,
+        normal_pat: &crate::pat::DeconstructedPat<Self>,
+    ) -> Self::Error {
+        let deref_pattern_label = deref_pat.data().span;
+        let normal_constructor_label = normal_pat.data().span;
+        self.tcx.dcx().emit_err(errors::MixedDerefPatternConstructors {
+            spans: vec![deref_pattern_label, normal_constructor_label],
+            smart_pointer_ty: deref_pat.ty().inner(),
+            deref_pattern_label,
+            normal_constructor_label,
+        })
+    }
 }
 
 /// Recursively expand this pattern into its subpatterns. Only useful for or-patterns.
@@ -1055,13 +1086,6 @@ pub fn analyze_match<'p, '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,
@@ -1080,48 +1104,3 @@ 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(())
-}
diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs
index c348cd508f9..b1c646e9884 100644
--- a/compiler/rustc_pattern_analysis/src/usefulness.rs
+++ b/compiler/rustc_pattern_analysis/src/usefulness.rs
@@ -720,7 +720,7 @@ use tracing::{debug, instrument};
 use self::PlaceValidity::*;
 use crate::constructor::{Constructor, ConstructorSet, IntRange};
 use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat};
-use crate::{MatchArm, PatCx, PrivateUninhabitedField};
+use crate::{MatchArm, PatCx, PrivateUninhabitedField, checks};
 #[cfg(not(feature = "rustc"))]
 pub fn ensure_sufficient_stack<R>(f: impl FnOnce() -> R) -> R {
     f()
@@ -1836,6 +1836,11 @@ pub fn compute_match_usefulness<'p, Cx: PatCx>(
     scrut_validity: PlaceValidity,
     complexity_limit: usize,
 ) -> Result<UsefulnessReport<'p, Cx>, Cx::Error> {
+    // The analysis doesn't support deref patterns mixed with normal constructors; error if present.
+    if tycx.match_may_contain_deref_pats() {
+        checks::detect_mixed_deref_pat_ctors(tycx, arms)?;
+    }
+
     let mut cx = UsefulnessCtxt {
         tycx,
         branch_usefulness: FxHashMap::default(),
diff --git a/compiler/rustc_pattern_analysis/tests/common/mod.rs b/compiler/rustc_pattern_analysis/tests/common/mod.rs
index 8980b644f59..0b939ef7816 100644
--- a/compiler/rustc_pattern_analysis/tests/common/mod.rs
+++ b/compiler/rustc_pattern_analysis/tests/common/mod.rs
@@ -1,6 +1,7 @@
 use rustc_pattern_analysis::constructor::{
     Constructor, ConstructorSet, IntRange, MaybeInfiniteInt, RangeEnd, VariantVisibility,
 };
+use rustc_pattern_analysis::pat::DeconstructedPat;
 use rustc_pattern_analysis::usefulness::{PlaceValidity, UsefulnessReport};
 use rustc_pattern_analysis::{MatchArm, PatCx, PrivateUninhabitedField};
 
@@ -184,6 +185,14 @@ impl PatCx for Cx {
     fn complexity_exceeded(&self) -> Result<(), Self::Error> {
         Err(())
     }
+
+    fn report_mixed_deref_pat_ctors(
+        &self,
+        _deref_pat: &DeconstructedPat<Self>,
+        _normal_pat: &DeconstructedPat<Self>,
+    ) -> Self::Error {
+        panic!("`rustc_pattern_analysis::tests` currently doesn't test deref pattern errors")
+    }
 }
 
 /// Construct a single pattern; see `pats!()`.
diff --git a/src/tools/rust-analyzer/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs b/src/tools/rust-analyzer/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs
index 22b7f5ac9fd..56fd12e1f2b 100644
--- a/src/tools/rust-analyzer/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs
+++ b/src/tools/rust-analyzer/crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs
@@ -489,6 +489,14 @@ impl PatCx for MatchCheckCtx<'_> {
     fn complexity_exceeded(&self) -> Result<(), Self::Error> {
         Err(())
     }
+
+    fn report_mixed_deref_pat_ctors(
+        &self,
+        _deref_pat: &DeconstructedPat<'_>,
+        _normal_pat: &DeconstructedPat<'_>,
+    ) {
+        // FIXME(deref_patterns): This could report an error comparable to the one in rustc.
+    }
 }
 
 impl fmt::Debug for MatchCheckCtx<'_> {
diff --git a/tests/ui/pattern/box-pattern-constructor-mismatch.rs b/tests/ui/pattern/box-pattern-constructor-mismatch.rs
new file mode 100644
index 00000000000..8f0a19d7407
--- /dev/null
+++ b/tests/ui/pattern/box-pattern-constructor-mismatch.rs
@@ -0,0 +1,11 @@
+//! Test that `box _` patterns and `Box { .. }` patterns can't be used to match on the same place.
+//! This is required for the current implementation of exhaustiveness analysis for deref patterns.
+
+#![feature(box_patterns)]
+
+fn main() {
+    match Box::new(0) {
+        box _ => {} //~ ERROR mix of deref patterns and normal constructors
+        Box { .. } => {}
+    }
+}
diff --git a/tests/ui/pattern/box-pattern-constructor-mismatch.stderr b/tests/ui/pattern/box-pattern-constructor-mismatch.stderr
new file mode 100644
index 00000000000..489eefe0d21
--- /dev/null
+++ b/tests/ui/pattern/box-pattern-constructor-mismatch.stderr
@@ -0,0 +1,10 @@
+error: mix of deref patterns and normal constructors
+  --> $DIR/box-pattern-constructor-mismatch.rs:8:9
+   |
+LL |         box _ => {}
+   |         ^^^^^ matches on the result of dereferencing `Box<i32>`
+LL |         Box { .. } => {}
+   |         ^^^^^^^^^^ matches directly on `Box<i32>`
+
+error: aborting due to 1 previous error
+