Auto merge of rust-lang#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#140106, deref pattern validation was tied to the `deref_patterns` feature to temporarily avoid affecting perf. However:
- As of rust-lang#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`

Author: bors

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) {

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(),
         }
     }
 

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(())
+}

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.

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(())
-}

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(),

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!()`.

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<'_> {

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 { .. } => {}
+    }
+}

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
+