Auto merge of #119835 - Nadrieril:simplify-empty-logic, r=<try>

Exhaustiveness: simplify empty pattern logic

The logic that handles empty patterns had gotten quite convoluted. This PR simplifies it a lot. I tried to make the logic as easy as possible to follow; this only does logically equivalent changes.

The first commit is a drive-by comment clarification that was requested after another PR a while back.

r? `@compiler-errors`

Author: bors

compiler/rustc_mir_build/src/thir/pattern/check_match.rs

@@ -289,7 +289,8 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
     fn is_known_valid_scrutinee(&self, scrutinee: &Expr<'tcx>) -> bool {
         use ExprKind::*;
         match &scrutinee.kind {
-            // Both pointers and references can validly point to a place with invalid data.
+            // Pointers can validly point to a place with invalid data. It is undecided whether
+            // references can too, so we conservatively assume they can.
             Deref { .. } => false,
             // Inherit validity of the parent place, unless the parent is an union.
             Field { lhs, .. } => {

compiler/rustc_pattern_analysis/src/constructor.rs

@@ -858,12 +858,14 @@ impl<Cx: TypeCx> ConstructorSet<Cx> {
     /// any) are missing; 2/ split constructors to handle non-trivial intersections e.g. on ranges
     /// or slices. This can get subtle; see [`SplitConstructorSet`] for details of this operation
     /// and its invariants.
-    #[instrument(level = "debug", skip(self, pcx, ctors), ret)]
+    #[instrument(level = "debug", skip(self, ctors), ret)]
     pub(crate) fn split<'a>(
         &self,
-        pcx: &PlaceCtxt<'a, Cx>,
         ctors: impl Iterator<Item = &'a Constructor<Cx>> + Clone,
-    ) -> SplitConstructorSet<Cx> {
+    ) -> SplitConstructorSet<Cx>
+    where
+        Cx: 'a,
+    {
         let mut present: SmallVec<[_; 1]> = SmallVec::new();
         // Empty constructors found missing.
         let mut missing_empty = Vec::new();
@@ -1003,17 +1005,6 @@ impl<Cx: TypeCx> ConstructorSet<Cx> {
             }
         }
 
-        // We have now grouped all the constructors into 3 buckets: present, missing, missing_empty.
-        // In the absence of the `exhaustive_patterns` feature however, we don't count nested empty
-        // types as empty. Only non-nested `!` or `enum Foo {}` are considered empty.
-        if !pcx.mcx.tycx.is_exhaustive_patterns_feature_on()
-            && !(pcx.is_scrutinee && matches!(self, Self::NoConstructors))
-        {
-            // Treat all missing constructors as nonempty.
-            // This clears `missing_empty`.
-            missing.append(&mut missing_empty);
-        }
-
         SplitConstructorSet { present, missing, missing_empty }
     }
 }

compiler/rustc_pattern_analysis/src/lints.rs

@@ -59,7 +59,7 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> {
     ) -> Result<SplitConstructorSet<'p, 'tcx>, ErrorGuaranteed> {
         let column_ctors = self.patterns.iter().map(|p| p.ctor());
         let ctors_for_ty = &pcx.ctors_for_ty()?;
-        Ok(ctors_for_ty.split(pcx, column_ctors))
+        Ok(ctors_for_ty.split(column_ctors))
     }
 
     /// Does specialization: given a constructor, this takes the patterns from the column that match

compiler/rustc_pattern_analysis/src/usefulness.rs

@@ -737,15 +737,13 @@ pub(crate) struct PlaceCtxt<'a, Cx: TypeCx> {
     pub(crate) mcx: MatchCtxt<'a, Cx>,
     /// Type of the place under investigation.
     pub(crate) ty: Cx::Ty,
-    /// Whether the place is the original scrutinee place, as opposed to a subplace of it.
-    pub(crate) is_scrutinee: bool,
 }
 
 impl<'a, Cx: TypeCx> PlaceCtxt<'a, Cx> {
     /// A `PlaceCtxt` when code other than `is_useful` needs one.
     #[cfg_attr(not(feature = "rustc"), allow(dead_code))]
     pub(crate) fn new_dummy(mcx: MatchCtxt<'a, Cx>, ty: Cx::Ty) -> Self {
-        PlaceCtxt { mcx, ty, is_scrutinee: false }
+        PlaceCtxt { mcx, ty }
     }
 
     pub(crate) fn ctor_arity(&self, ctor: &Constructor<Cx>) -> usize {
@@ -768,30 +766,16 @@ impl<'a, Cx: TypeCx> PlaceCtxt<'a, Cx> {
 pub enum ValidityConstraint {
     ValidOnly,
     MaybeInvalid,
-    /// Option for backwards compatibility: the place is not known to be valid but we allow omitting
-    /// `useful && !reachable` arms anyway.
-    MaybeInvalidButAllowOmittingArms,
 }
 
 impl ValidityConstraint {
     pub fn from_bool(is_valid_only: bool) -> Self {
         if is_valid_only { ValidOnly } else { MaybeInvalid }
     }
 
-    fn allow_omitting_side_effecting_arms(self) -> Self {
-        match self {
-            MaybeInvalid | MaybeInvalidButAllowOmittingArms => MaybeInvalidButAllowOmittingArms,
-            // There are no side-effecting empty arms here, nothing to do.
-            ValidOnly => ValidOnly,
-        }
-    }
-
     fn is_known_valid(self) -> bool {
         matches!(self, ValidOnly)
     }
-    fn allows_omitting_empty_arms(self) -> bool {
-        matches!(self, ValidOnly | MaybeInvalidButAllowOmittingArms)
-    }
 
     /// If the place has validity given by `self` and we read that the value at the place has
     /// constructor `ctor`, this computes what we can assume about the validity of the constructor
@@ -814,7 +798,7 @@ impl fmt::Display for ValidityConstraint {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let s = match self {
             ValidOnly => "✓",
-            MaybeInvalid | MaybeInvalidButAllowOmittingArms => "?",
+            MaybeInvalid => "?",
         };
         write!(f, "{s}")
     }
@@ -1456,41 +1440,44 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
     };
 
     debug!("ty: {ty:?}");
-    let pcx = &PlaceCtxt { mcx, ty, is_scrutinee: is_top_level };
+    let pcx = &PlaceCtxt { mcx, ty };
+    let ctors_for_ty = pcx.ctors_for_ty()?;
 
     // Whether the place/column we are inspecting is known to contain valid data.
     let place_validity = matrix.place_validity[0];
-    // For backwards compability we allow omitting some empty arms that we ideally shouldn't.
-    let place_validity = place_validity.allow_omitting_side_effecting_arms();
+    // We treat match scrutinees of type `!` or `EmptyEnum` differently.
+    let is_toplevel_exception =
+        is_top_level && matches!(ctors_for_ty, ConstructorSet::NoConstructors);
+    // Whether empty patterns can be omitted for exhaustiveness.
+    let can_omit_empty_arms = is_toplevel_exception || mcx.tycx.is_exhaustive_patterns_feature_on();
+    // Whether empty patterns are counted as useful or not.
+    let empty_arms_are_unreachable = place_validity.is_known_valid() && can_omit_empty_arms;
 
     // Analyze the constructors present in this column.
     let ctors = matrix.heads().map(|p| p.ctor());
-    let ctors_for_ty = pcx.ctors_for_ty()?;
-    let is_integers = matches!(ctors_for_ty, ConstructorSet::Integers { .. }); // For diagnostics.
-    let split_set = ctors_for_ty.split(pcx, ctors);
+    let mut split_set = ctors_for_ty.split(ctors);
     let all_missing = split_set.present.is_empty();
-
     // Build the set of constructors we will specialize with. It must cover the whole type.
+    // We need to iterate over a full set of constructors, so we add `Missing` to represent the
+    // missing ones. This is explained under "Constructor Splitting" at the top of this file.
     let mut split_ctors = split_set.present;
-    if !split_set.missing.is_empty() {
-        // We need to iterate over a full set of constructors, so we add `Missing` to represent the
-        // missing ones. This is explained under "Constructor Splitting" at the top of this file.
-        split_ctors.push(Constructor::Missing);
-    } else if !split_set.missing_empty.is_empty() && !place_validity.is_known_valid() {
-        // The missing empty constructors are reachable if the place can contain invalid data.
+    if !(split_set.missing.is_empty()
+        && (split_set.missing_empty.is_empty() || empty_arms_are_unreachable))
+    {
         split_ctors.push(Constructor::Missing);
     }
 
     // Decide what constructors to report.
+    let is_integers = matches!(ctors_for_ty, ConstructorSet::Integers { .. });
     let always_report_all = is_top_level && !is_integers;
     // Whether we should report "Enum::A and Enum::C are missing" or "_ is missing".
     let report_individual_missing_ctors = always_report_all || !all_missing;
     // Which constructors are considered missing. We ensure that `!missing_ctors.is_empty() =>
-    // split_ctors.contains(Missing)`. The converse usually holds except in the
-    // `MaybeInvalidButAllowOmittingArms` backwards-compatibility case.
+    // split_ctors.contains(Missing)`. The converse usually holds except when
+    // `!place_validity.is_known_valid()`.
     let mut missing_ctors = split_set.missing;
-    if !place_validity.allows_omitting_empty_arms() {
-        missing_ctors.extend(split_set.missing_empty);
+    if !can_omit_empty_arms {
+        missing_ctors.append(&mut split_set.missing_empty);
     }
 
     let mut ret = WitnessMatrix::empty();