Let WitnessMatrix decide what to report

Author: Nadrieril

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

@@ -700,15 +700,16 @@ impl<'tcx> WitnessMatrix<'tcx> {
         pcx: &PatCtxt<'_, '_, 'tcx>,
         missing_ctors: &[Constructor<'tcx>],
         ctor: &Constructor<'tcx>,
+        report_when_all_missing: bool,
     ) {
         if self.is_empty() {
             return;
         }
-        if matches!(ctor, Constructor::Missing { .. }) {
-            // We got the special `Missing` constructor, so each of the missing constructors gives a
-            // new pattern that is not caught by the match. We list those patterns and push them
-            // onto our current witnesses.
-            if missing_ctors.iter().any(|c| c.is_non_exhaustive()) {
+        if matches!(ctor, Constructor::Wildcard | Constructor::Missing) {
+            if matches!(ctor, Constructor::Wildcard) && !report_when_all_missing {
+                let pat = WitnessPat::wild_from_ctor(pcx, Constructor::Wildcard);
+                self.push_pattern(&pat);
+            } else if missing_ctors.iter().any(|c| c.is_non_exhaustive()) {
                 // We only report `_` here; listing other constructors would be redundant.
                 let pat = WitnessPat::wild_from_ctor(pcx, Constructor::NonExhaustive);
                 self.push_pattern(&pat);
@@ -722,8 +723,17 @@ impl<'tcx> WitnessMatrix<'tcx> {
                 }
             }
         } else {
-            for witness in self.0.iter_mut() {
-                witness.apply_constructor(pcx, ctor)
+            // We have a non-wildcard constructor.
+            if !missing_ctors.is_empty() {
+                // If some ctors are missing we want to only report them (so we discard non-missing
+                // ones). This is guaranteed to report *some* witnesses because
+                // `!missing_ctors.is_empty()` implies there is a `Missing` or `Wildcard` in
+                // `split_ctors`.
+                *self = Self::new_empty();
+            } else {
+                for witness in self.0.iter_mut() {
+                    witness.apply_constructor(pcx, ctor)
+                }
             }
         }
     }
@@ -760,7 +770,6 @@ impl<'tcx> WitnessMatrix<'tcx> {
 fn compute_usefulness<'p, 'tcx>(
     cx: &MatchCheckCtxt<'p, 'tcx>,
     matrix: &mut Matrix<'p, 'tcx>,
-    collect_witnesses: bool,
     lint_root: HirId,
     is_top_level: bool,
 ) -> WitnessMatrix<'tcx> {
@@ -777,7 +786,7 @@ fn compute_usefulness<'p, 'tcx>(
                 break;
             }
         }
-        if useful && collect_witnesses {
+        if useful {
             return WitnessMatrix::new_unit();
         } else {
             return WitnessMatrix::new_empty();
@@ -792,59 +801,17 @@ fn compute_usefulness<'p, 'tcx>(
     let split_set = ConstructorSet::for_ty(pcx.cx, pcx.ty).split(pcx, ctors, is_top_level);
     let mut split_ctors = split_set.present;
     let missing_ctors = split_set.missing;
+    // We want to iterate over a full set of constructors, so if any is missing we add a wildcard.
     if !missing_ctors.is_empty() {
-        // We are splitting a wildcard in order to compute its usefulness. Some constructors are
-        // not present in the column. The first thing we note is that specializing with any of
-        // the missing constructors would select exactly the rows with wildcards. Moreover, they
-        // would all return equivalent results. We can therefore group them all into a
-        // fictitious `Missing` constructor.
-        //
-        // As an important optimization, this function will skip all the present constructors.
-        // This is correct because specializing with any of the present constructors would
-        // select a strict superset of the wildcard rows, and thus would only find witnesses
-        // already found with the `Missing` constructor.
-        // This does mean that diagnostics are incomplete: in
-        // ```
-        // match x {
-        //   Some(true) => {}
-        // }
-        // ```
-        // we report `None` as missing but not `Some(false)`.
-        //
-        // When all the constructors are missing we can equivalently return the `Wildcard`
-        // constructor on its own. The difference between `Wildcard` and `Missing` will then
-        // only be in diagnostics.
-
-        // If some constructors are missing, we typically want to report those constructors,
-        // e.g.:
-        // ```
-        //     enum Direction { N, S, E, W }
-        //     let Direction::N = ...;
-        // ```
-        // we can report 3 witnesses: `S`, `E`, and `W`.
-        //
-        // However, if the user didn't actually specify a constructor
-        // in this arm, e.g., in
-        // ```
-        //     let x: (Direction, Direction, bool) = ...;
-        //     let (_, _, false) = x;
-        // ```
-        // we don't want to show all 16 possible witnesses `(<direction-1>, <direction-2>,
-        // true)` - we are satisfied with `(_, _, true)`. So if all constructors are missing we
-        // prefer to report just a wildcard `_`.
-        //
-        // The exception is: if we are at the top-level, for example in an empty match, we
-        // usually prefer to report the full list of constructors.
-        let all_missing = split_ctors.is_empty();
-        let report_when_all_missing =
-            is_top_level && !super::deconstruct_pat::IntRange::is_integral(pcx.ty);
-        let ctor = if all_missing && !report_when_all_missing {
-            Constructor::Wildcard
+        if split_ctors.is_empty() {
+            split_ctors.push(Constructor::Wildcard);
         } else {
-            Constructor::Missing
-        };
-        split_ctors.push(ctor);
+            split_ctors.push(Constructor::Missing);
+        }
     }
+    // At the top level, we prefer to list all constructors when `_` could be reported as missing.
+    let report_when_all_missing =
+        is_top_level && !super::deconstruct_pat::IntRange::is_integral(pcx.ty);
 
     if super::deconstruct_pat::IntRange::is_integral(ty) {
         for row_id in 0..matrix.len() {
@@ -874,20 +841,12 @@ fn compute_usefulness<'p, 'tcx>(
     // witness the usefulness of `v`.
     let mut ret = WitnessMatrix::new_empty();
     for ctor in split_ctors {
-        // If some ctors are missing we only report those. Could report all if that's useful for
-        // some applications.
-        let collect_witnesses = collect_witnesses
-            && (missing_ctors.is_empty()
-                || matches!(ctor, Constructor::Wildcard | Constructor::Missing));
         debug!("specialize({:?})", ctor);
         let mut spec_matrix = matrix.specialize_constructor(pcx, &ctor);
-        let mut witnesses = ensure_sufficient_stack(|| {
-            compute_usefulness(cx, &mut spec_matrix, collect_witnesses, lint_root, false)
-        });
-        if collect_witnesses {
-            witnesses.apply_constructor(pcx, &missing_ctors, &ctor);
-            ret.extend(witnesses);
-        }
+        let mut witnesses =
+            ensure_sufficient_stack(|| compute_usefulness(cx, &mut spec_matrix, lint_root, false));
+        witnesses.apply_constructor(pcx, &missing_ctors, &ctor, report_when_all_missing);
+        ret.extend(witnesses);
 
         for child_row in spec_matrix.rows() {
             let parent_row = &mut matrix.rows[child_row.parent_row];
@@ -1028,7 +987,7 @@ pub(crate) fn compute_match_usefulness<'p, 'tcx>(
         matrix.push(v);
     }
 
-    let non_exhaustiveness_witnesses = compute_usefulness(cx, &mut matrix, true, lint_root, true);
+    let non_exhaustiveness_witnesses = compute_usefulness(cx, &mut matrix, lint_root, true);
     let non_exhaustiveness_witnesses: Vec<_> = non_exhaustiveness_witnesses.single_column();
     let arm_usefulness: Vec<_> = arms
         .iter()