Skip to content

Commit

Permalink
Auto merge of rust-lang#128034 - Nadrieril:explain-unreachable, r=<try>
Browse files Browse the repository at this point in the history
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 rust-lang#127870. This is also the other one of the two diagnostic improvements I wanted to do before rust-lang#122792.

Note: the first commit is rust-lang#128015, the second is an unrelated drive-by tweak.

r? `@compiler-errors`
  • Loading branch information
bors committed Jul 21, 2024
2 parents 92c6c03 + fbc2f03 commit 49ee7bc
Show file tree
Hide file tree
Showing 53 changed files with 1,468 additions and 400 deletions.
5 changes: 4 additions & 1 deletion compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,10 @@ mir_build_union_pattern = cannot use unions in constant patterns
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 = matches some of the same values
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
Expand Down
32 changes: 29 additions & 3 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,11 +582,37 @@ 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>,
#[subdiagnostic]
pub(crate) covered_by_many: Option<UnreachableCoveredByMany>,
}

#[derive(Subdiagnostic)]
#[note(mir_build_unreachable_matches_no_values)]
pub(crate) struct UnreachableMatchesNoValues<'tcx> {
pub(crate) ty: Ty<'tcx>,
}

pub(crate) struct UnreachableCoveredByMany(pub(crate) Vec<Span>);

impl Subdiagnostic for UnreachableCoveredByMany {
fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
self,
diag: &mut Diag<'_, G>,
_f: &F,
) {
for span in self.0 {
diag.span_label(span, fluent::mir_build_unreachable_covered_by_many);
}
}
}

#[derive(Diagnostic)]
Expand Down
80 changes: 55 additions & 25 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,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, RustcPatCtxt as PatCtxt, Usefulness, UsefulnessReport,
WitnessPat,
Constructor, DeconstructedPat, MatchArm, RedundancyExplanation, RustcPatCtxt as PatCtxt,
Usefulness, UsefulnessReport, WitnessPat,
};
use rustc_session::lint::builtin::{
BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
Expand Down Expand Up @@ -391,12 +391,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() {
Expand All @@ -405,9 +409,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)
}
}
}
Expand Down Expand Up @@ -906,26 +910,52 @@ 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 covering_spans = covering_pats.iter().map(|p| p.data().span).collect();
lint.covered_by_many = Some(UnreachableCoveredByMany(covering_spans));
}
}
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)
}
}
}
Expand Down
37 changes: 3 additions & 34 deletions compiler/rustc_pattern_analysis/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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 {}
Expand Down Expand Up @@ -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)
}
30 changes: 28 additions & 2 deletions compiler/rustc_pattern_analysis/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
///
Expand Down Expand Up @@ -190,7 +205,18 @@ impl<'p, Cx: PatCx> PatOrWild<'p, Cx> {
}
}

/// Expand this (possibly-nested) or-pattern into its alternatives.
/// Expand this or-pattern into its alternatives. This only expands one or-pattern; use
/// `flatten_or_pat` to recursively expand nested or-patterns.
pub(crate) fn expand_or_pat(self) -> SmallVec<[Self; 1]> {
match self {
PatOrWild::Pat(pat) if pat.is_or_pat() => {
pat.iter_fields().map(|ipat| PatOrWild::Pat(&ipat.pat)).collect()
}
_ => smallvec![self],
}
}

/// Recursively expand this (possibly-nested) or-pattern into its alternatives.
pub(crate) fn flatten_or_pat(self) -> SmallVec<[Self; 1]> {
match self {
PatOrWild::Pat(pat) if pat.is_or_pat() => pat
Expand Down
28 changes: 28 additions & 0 deletions compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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>>;
Expand Down Expand Up @@ -1052,3 +1057,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)
}
Loading

0 comments on commit 49ee7bc

Please sign in to comment.