Auto merge of #128034 - Nadrieril:explain-unreachable, r=compiler-errors

exhaustiveness: Explain why a given pattern is considered unreachable

This PR tells the user why a given pattern is considered unreachable. I reused the intersection information we were already computing; even though it's incomplete I convinced myself that it is sufficient to always get a set of patterns that cover the unreachable one.

I'm not a fan of the diagnostic messages I came up with, I'm open to suggestions.

Fixes #127870. This is also the other one of the two diagnostic improvements I wanted to do before #122792.

Note: the first commit is an unrelated drive-by tweak.

r? `@compiler-errors`

Author: bors

Diff for: compiler/rustc_mir_build/messages.ftl

@@ -325,9 +325,16 @@ mir_build_union_field_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
 
 mir_build_union_pattern = cannot use unions in constant patterns
 
+mir_build_unreachable_making_this_unreachable = collectively making this unreachable
+
+mir_build_unreachable_matches_same_values = matches some of the same values
+
 mir_build_unreachable_pattern = unreachable pattern
     .label = unreachable pattern
-    .catchall_label = matches any value
+    .unreachable_matches_no_values = this pattern matches no values because `{$ty}` is uninhabited
+    .unreachable_covered_by_catchall = matches any value
+    .unreachable_covered_by_one = matches all the values already
+    .unreachable_covered_by_many = these patterns collectively make the last one unreachable
 
 mir_build_unsafe_fn_safe_body = an unsafe function restricts its caller, but its body is safe by default
 mir_build_unsafe_not_inherited = items do not inherit unsafety from separate enclosing items

Diff for: compiler/rustc_mir_build/src/errors.rs

@@ -582,11 +582,23 @@ pub(crate) struct NonConstPath {
 
 #[derive(LintDiagnostic)]
 #[diag(mir_build_unreachable_pattern)]
-pub(crate) struct UnreachablePattern {
+pub(crate) struct UnreachablePattern<'tcx> {
     #[label]
     pub(crate) span: Option<Span>,
-    #[label(mir_build_catchall_label)]
-    pub(crate) catchall: Option<Span>,
+    #[subdiagnostic]
+    pub(crate) matches_no_values: Option<UnreachableMatchesNoValues<'tcx>>,
+    #[label(mir_build_unreachable_covered_by_catchall)]
+    pub(crate) covered_by_catchall: Option<Span>,
+    #[label(mir_build_unreachable_covered_by_one)]
+    pub(crate) covered_by_one: Option<Span>,
+    #[note(mir_build_unreachable_covered_by_many)]
+    pub(crate) covered_by_many: Option<MultiSpan>,
+}
+
+#[derive(Subdiagnostic)]
+#[note(mir_build_unreachable_matches_no_values)]
+pub(crate) struct UnreachableMatchesNoValues<'tcx> {
+    pub(crate) ty: Ty<'tcx>,
 }
 
 #[derive(Diagnostic)]

Diff for: compiler/rustc_mir_build/src/thir/pattern/check_match.rs

@@ -1,4 +1,5 @@
 use crate::errors::*;
+use crate::fluent_generated as fluent;
 
 use rustc_arena::{DroplessArena, TypedArena};
 use rustc_ast::Mutability;
@@ -16,8 +17,8 @@ use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt};
 use rustc_pattern_analysis::errors::Uncovered;
 use rustc_pattern_analysis::rustc::{
-    Constructor, DeconstructedPat, MatchArm, RevealedTy, RustcPatCtxt as PatCtxt, Usefulness,
-    UsefulnessReport, WitnessPat,
+    Constructor, DeconstructedPat, MatchArm, RedundancyExplanation, RevealedTy,
+    RustcPatCtxt as PatCtxt, Usefulness, UsefulnessReport, WitnessPat,
 };
 use rustc_session::lint::builtin::{
     BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
@@ -391,12 +392,16 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
     ) -> Result<UsefulnessReport<'p, 'tcx>, ErrorGuaranteed> {
         let pattern_complexity_limit =
             get_limit_size(cx.tcx.hir().krate_attrs(), cx.tcx.sess, sym::pattern_complexity);
-        let report =
-            rustc_pattern_analysis::analyze_match(&cx, &arms, scrut_ty, pattern_complexity_limit)
-                .map_err(|err| {
-                self.error = Err(err);
-                err
-            })?;
+        let report = rustc_pattern_analysis::rustc::analyze_match(
+            &cx,
+            &arms,
+            scrut_ty,
+            pattern_complexity_limit,
+        )
+        .map_err(|err| {
+            self.error = Err(err);
+            err
+        })?;
 
         // Warn unreachable subpatterns.
         for (arm, is_useful) in report.arm_usefulness.iter() {
@@ -405,9 +410,9 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
             {
                 let mut redundant_subpats = redundant_subpats.clone();
                 // Emit lints in the order in which they occur in the file.
-                redundant_subpats.sort_unstable_by_key(|pat| pat.data().span);
-                for pat in redundant_subpats {
-                    report_unreachable_pattern(cx, arm.arm_data, pat.data().span, None)
+                redundant_subpats.sort_unstable_by_key(|(pat, _)| pat.data().span);
+                for (pat, explanation) in redundant_subpats {
+                    report_unreachable_pattern(cx, arm.arm_data, pat, &explanation)
                 }
             }
         }
@@ -906,26 +911,60 @@ fn report_irrefutable_let_patterns(
 fn report_unreachable_pattern<'p, 'tcx>(
     cx: &PatCtxt<'p, 'tcx>,
     hir_id: HirId,
-    span: Span,
-    catchall: Option<Span>,
+    pat: &DeconstructedPat<'p, 'tcx>,
+    explanation: &RedundancyExplanation<'p, 'tcx>,
 ) {
-    cx.tcx.emit_node_span_lint(
-        UNREACHABLE_PATTERNS,
-        hir_id,
-        span,
-        UnreachablePattern { span: if catchall.is_some() { Some(span) } else { None }, catchall },
-    );
+    let pat_span = pat.data().span;
+    let mut lint = UnreachablePattern {
+        span: Some(pat_span),
+        matches_no_values: None,
+        covered_by_catchall: None,
+        covered_by_one: None,
+        covered_by_many: None,
+    };
+    match explanation.covered_by.as_slice() {
+        [] => {
+            // Empty pattern; we report the uninhabited type that caused the emptiness.
+            lint.span = None; // Don't label the pattern itself
+            pat.walk(&mut |subpat| {
+                let ty = **subpat.ty();
+                if cx.is_uninhabited(ty) {
+                    lint.matches_no_values = Some(UnreachableMatchesNoValues { ty });
+                    false // No need to dig further.
+                } else if matches!(subpat.ctor(), Constructor::Ref | Constructor::UnionField) {
+                    false // Don't explore further since they are not by-value.
+                } else {
+                    true
+                }
+            });
+        }
+        [covering_pat] if pat_is_catchall(covering_pat) => {
+            lint.covered_by_catchall = Some(covering_pat.data().span);
+        }
+        [covering_pat] => {
+            lint.covered_by_one = Some(covering_pat.data().span);
+        }
+        covering_pats => {
+            let mut multispan = MultiSpan::from_span(pat_span);
+            for p in covering_pats {
+                multispan.push_span_label(
+                    p.data().span,
+                    fluent::mir_build_unreachable_matches_same_values,
+                );
+            }
+            multispan
+                .push_span_label(pat_span, fluent::mir_build_unreachable_making_this_unreachable);
+            lint.covered_by_many = Some(multispan);
+        }
+    }
+    cx.tcx.emit_node_span_lint(UNREACHABLE_PATTERNS, hir_id, pat_span, lint);
 }
 
 /// Report unreachable arms, if any.
 fn report_arm_reachability<'p, 'tcx>(cx: &PatCtxt<'p, 'tcx>, report: &UsefulnessReport<'p, 'tcx>) {
-    let mut catchall = None;
     for (arm, is_useful) in report.arm_usefulness.iter() {
-        if matches!(is_useful, Usefulness::Redundant) {
-            report_unreachable_pattern(cx, arm.arm_data, arm.pat.data().span, catchall)
-        }
-        if !arm.has_guard && catchall.is_none() && pat_is_catchall(arm.pat) {
-            catchall = Some(arm.pat.data().span);
+        if let Usefulness::Redundant(explanation) = is_useful {
+            report_unreachable_pattern(cx, arm.arm_data, arm.pat, explanation)
         }
     }
 }

Diff for: compiler/rustc_pattern_analysis/src/lib.rs

@@ -1,4 +1,6 @@
-//! Analysis of patterns, notably match exhaustiveness checking.
+//! Analysis of patterns, notably match exhaustiveness checking. The main entrypoint for this crate
+//! is [`usefulness::compute_match_usefulness`]. For rustc-specific types and entrypoints, see the
+//! [`rustc`] module.
 
 // tidy-alphabetical-start
 #![allow(rustc::diagnostic_outside_of_impl)]
@@ -23,14 +25,8 @@ use std::fmt;
 
 pub use rustc_index::{Idx, IndexVec}; // re-exported to avoid rustc_index version issues
 
-#[cfg(feature = "rustc")]
-use rustc_middle::ty::Ty;
-#[cfg(feature = "rustc")]
-use rustc_span::ErrorGuaranteed;
-
 use crate::constructor::{Constructor, ConstructorSet, IntRange};
 use crate::pat::DeconstructedPat;
-use crate::pat_column::PatternColumn;
 
 pub trait Captures<'a> {}
 impl<'a, T: ?Sized> Captures<'a> for T {}
@@ -128,30 +124,3 @@ impl<'p, Cx: PatCx> Clone for MatchArm<'p, Cx> {
 }
 
 impl<'p, Cx: PatCx> Copy for MatchArm<'p, Cx> {}
-
-/// The entrypoint for this crate. Computes whether a match is exhaustive and which of its arms are
-/// useful, and runs some lints.
-#[cfg(feature = "rustc")]
-pub fn analyze_match<'p, 'tcx>(
-    tycx: &rustc::RustcPatCtxt<'p, 'tcx>,
-    arms: &[rustc::MatchArm<'p, 'tcx>],
-    scrut_ty: Ty<'tcx>,
-    pattern_complexity_limit: Option<usize>,
-) -> Result<rustc::UsefulnessReport<'p, 'tcx>, ErrorGuaranteed> {
-    use lints::lint_nonexhaustive_missing_variants;
-    use usefulness::{compute_match_usefulness, PlaceValidity};
-
-    let scrut_ty = tycx.reveal_opaque_ty(scrut_ty);
-    let scrut_validity = PlaceValidity::from_bool(tycx.known_valid_scrutinee);
-    let report =
-        compute_match_usefulness(tycx, arms, scrut_ty, scrut_validity, pattern_complexity_limit)?;
-
-    // Run the non_exhaustive_omitted_patterns lint. Only run on refutable patterns to avoid hitting
-    // `if let`s. Only run if the match is exhaustive otherwise the error is redundant.
-    if tycx.refutable && report.non_exhaustiveness_witnesses.is_empty() {
-        let pat_column = PatternColumn::new(arms);
-        lint_nonexhaustive_missing_variants(tycx, arms, &pat_column, scrut_ty)?;
-    }
-
-    Ok(report)
-}

Diff for: compiler/rustc_pattern_analysis/src/pat.rs

@@ -11,7 +11,7 @@ use crate::{PatCx, PrivateUninhabitedField};
 use self::Constructor::*;
 
 /// A globally unique id to distinguish patterns.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub(crate) struct PatId(u32);
 impl PatId {
     fn new() -> Self {
@@ -147,6 +147,21 @@ impl<Cx: PatCx> fmt::Debug for DeconstructedPat<Cx> {
     }
 }
 
+/// Delegate to `uid`.
+impl<Cx: PatCx> PartialEq for DeconstructedPat<Cx> {
+    fn eq(&self, other: &Self) -> bool {
+        self.uid == other.uid
+    }
+}
+/// Delegate to `uid`.
+impl<Cx: PatCx> Eq for DeconstructedPat<Cx> {}
+/// Delegate to `uid`.
+impl<Cx: PatCx> std::hash::Hash for DeconstructedPat<Cx> {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.uid.hash(state);
+    }
+}
+
 /// Represents either a pattern obtained from user input or a wildcard constructed during the
 /// algorithm. Do not use `Wild` to represent a wildcard pattern comping from user input.
 ///

Diff for: compiler/rustc_pattern_analysis/src/rustc.rs

@@ -20,6 +20,9 @@ use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT};
 use crate::constructor::{
     IntRange, MaybeInfiniteInt, OpaqueId, RangeEnd, Slice, SliceKind, VariantVisibility,
 };
+use crate::lints::lint_nonexhaustive_missing_variants;
+use crate::pat_column::PatternColumn;
+use crate::usefulness::{compute_match_usefulness, PlaceValidity};
 use crate::{errors, Captures, PatCx, PrivateUninhabitedField};
 
 use crate::constructor::Constructor::*;
@@ -29,6 +32,8 @@ pub type Constructor<'p, 'tcx> = crate::constructor::Constructor<RustcPatCtxt<'p
 pub type ConstructorSet<'p, 'tcx> = crate::constructor::ConstructorSet<RustcPatCtxt<'p, 'tcx>>;
 pub type DeconstructedPat<'p, 'tcx> = crate::pat::DeconstructedPat<RustcPatCtxt<'p, 'tcx>>;
 pub type MatchArm<'p, 'tcx> = crate::MatchArm<'p, RustcPatCtxt<'p, 'tcx>>;
+pub type RedundancyExplanation<'p, 'tcx> =
+    crate::usefulness::RedundancyExplanation<'p, RustcPatCtxt<'p, 'tcx>>;
 pub type Usefulness<'p, 'tcx> = crate::usefulness::Usefulness<'p, RustcPatCtxt<'p, 'tcx>>;
 pub type UsefulnessReport<'p, 'tcx> =
     crate::usefulness::UsefulnessReport<'p, RustcPatCtxt<'p, 'tcx>>;
@@ -1058,3 +1063,26 @@ fn expand_or_pat<'p, 'tcx>(pat: &'p Pat<'tcx>) -> Vec<&'p Pat<'tcx>> {
     expand(pat, &mut pats);
     pats
 }
+
+/// The entrypoint for this crate. Computes whether a match is exhaustive and which of its arms are
+/// useful, and runs some lints.
+pub fn analyze_match<'p, 'tcx>(
+    tycx: &RustcPatCtxt<'p, 'tcx>,
+    arms: &[MatchArm<'p, 'tcx>],
+    scrut_ty: Ty<'tcx>,
+    pattern_complexity_limit: Option<usize>,
+) -> Result<UsefulnessReport<'p, 'tcx>, ErrorGuaranteed> {
+    let scrut_ty = tycx.reveal_opaque_ty(scrut_ty);
+    let scrut_validity = PlaceValidity::from_bool(tycx.known_valid_scrutinee);
+    let report =
+        compute_match_usefulness(tycx, arms, scrut_ty, scrut_validity, pattern_complexity_limit)?;
+
+    // Run the non_exhaustive_omitted_patterns lint. Only run on refutable patterns to avoid hitting
+    // `if let`s. Only run if the match is exhaustive otherwise the error is redundant.
+    if tycx.refutable && report.non_exhaustiveness_witnesses.is_empty() {
+        let pat_column = PatternColumn::new(arms);
+        lint_nonexhaustive_missing_variants(tycx, arms, &pat_column, scrut_ty)?;
+    }
+
+    Ok(report)
+}