Skip to content

Commit

Permalink
Rollup merge of rust-lang#120097 - Nadrieril:consistent_unreachable_s…
Browse files Browse the repository at this point in the history
…ubpats, r=compiler-errors

Report unreachable subpatterns consistently

We weren't reporting unreachable subpatterns in function arguments and `let` expressions. This wasn't very important, but never patterns make it more relevant: a user might write `let (Ok(x) | Err(!)) = ...` in a case where `let Ok(x) = ...` is accepted, so we should report the `Err(!)` as redundant.

r? ``@compiler-errors``
  • Loading branch information
matthiaskrgr authored Jan 22, 2024
2 parents 071938a + 0a9bb97 commit 64cea92
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 40 deletions.
88 changes: 49 additions & 39 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use rustc_pattern_analysis::errors::Uncovered;
use rustc_pattern_analysis::rustc::{
Constructor, DeconstructedPat, RustcMatchCheckCtxt as MatchCheckCtxt, Usefulness,
Constructor, DeconstructedPat, MatchArm, RustcMatchCheckCtxt as MatchCheckCtxt, Usefulness,
UsefulnessReport, WitnessPat,
};
use rustc_pattern_analysis::{analyze_match, MatchArm};

use crate::errors::*;

Expand Down Expand Up @@ -390,6 +389,34 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
}
}

fn analyze_patterns(
&mut self,
cx: &MatchCheckCtxt<'p, 'tcx>,
arms: &[MatchArm<'p, 'tcx>],
scrut_ty: Ty<'tcx>,
) -> Result<UsefulnessReport<'p, 'tcx>, ErrorGuaranteed> {
let report =
rustc_pattern_analysis::analyze_match(&cx, &arms, scrut_ty).map_err(|err| {
self.error = Err(err);
err
})?;

// Warn unreachable subpatterns.
for (arm, is_useful) in report.arm_usefulness.iter() {
if let Usefulness::Useful(redundant_subpats) = is_useful
&& !redundant_subpats.is_empty()
{
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().unwrap().span);
for pat in redundant_subpats {
report_unreachable_pattern(cx, arm.arm_data, pat.data().unwrap().span, None)
}
}
}
Ok(report)
}

#[instrument(level = "trace", skip(self))]
fn check_let(&mut self, pat: &'p Pat<'tcx>, scrutinee: Option<ExprId>, span: Span) {
assert!(self.let_source != LetSource::None);
Expand Down Expand Up @@ -435,14 +462,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
}
}

let scrut_ty = scrut.ty;
let report = match analyze_match(&cx, &tarms, scrut_ty) {
Ok(report) => report,
Err(err) => {
self.error = Err(err);
return;
}
};
let Ok(report) = self.analyze_patterns(&cx, &tarms, scrut.ty) else { return };

match source {
// Don't report arm reachability of desugared `match $iter.into_iter() { iter => .. }`
Expand Down Expand Up @@ -474,7 +494,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
);
} else {
self.error = Err(report_non_exhaustive_match(
&cx, self.thir, scrut_ty, scrut.span, witnesses, arms, expr_span,
&cx, self.thir, scrut.ty, scrut.span, witnesses, arms, expr_span,
));
}
}
Expand Down Expand Up @@ -556,7 +576,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
let cx = self.new_cx(refutability, None, scrut, pat.span);
let pat = self.lower_pattern(&cx, pat)?;
let arms = [MatchArm { pat, arm_data: self.lint_level, has_guard: false }];
let report = analyze_match(&cx, &arms, pat.ty().inner())?;
let report = self.analyze_patterns(&cx, &arms, pat.ty().inner())?;
Ok((cx, report))
}

Expand All @@ -567,7 +587,6 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
) -> Result<RefutableFlag, ErrorGuaranteed> {
let (cx, report) = self.analyze_binding(pat, Refutable, scrut)?;
// Report if the pattern is unreachable, which can only occur when the type is uninhabited.
// This also reports unreachable sub-patterns.
report_arm_reachability(&cx, &report);
// If the list of witnesses is empty, the match is exhaustive, i.e. the `if let` pattern is
// irrefutable.
Expand Down Expand Up @@ -850,39 +869,30 @@ fn report_irrefutable_let_patterns(
}
}

/// Report unreachable arms, if any.
fn report_unreachable_pattern<'p, 'tcx>(
cx: &MatchCheckCtxt<'p, 'tcx>,
hir_id: HirId,
span: Span,
catchall: Option<Span>,
) {
cx.tcx.emit_spanned_lint(
UNREACHABLE_PATTERNS,
hir_id,
span,
UnreachablePattern { span: if catchall.is_some() { Some(span) } else { None }, catchall },
);
}

/// Report unreachable arms, if any.
fn report_arm_reachability<'p, 'tcx>(
cx: &MatchCheckCtxt<'p, 'tcx>,
report: &UsefulnessReport<'p, 'tcx>,
) {
let report_unreachable_pattern = |span, hir_id, catchall: Option<Span>| {
cx.tcx.emit_spanned_lint(
UNREACHABLE_PATTERNS,
hir_id,
span,
UnreachablePattern {
span: if catchall.is_some() { Some(span) } else { None },
catchall,
},
);
};

let mut catchall = None;
for (arm, is_useful) in report.arm_usefulness.iter() {
match is_useful {
Usefulness::Redundant => {
report_unreachable_pattern(arm.pat.data().unwrap().span, arm.arm_data, catchall)
}
Usefulness::Useful(redundant_subpats) if redundant_subpats.is_empty() => {}
// The arm is reachable, but contains redundant subpatterns (from or-patterns).
Usefulness::Useful(redundant_subpats) => {
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().unwrap().span);
for pat in redundant_subpats {
report_unreachable_pattern(pat.data().unwrap().span, arm.arm_data, None);
}
}
if matches!(is_useful, Usefulness::Redundant) {
report_unreachable_pattern(cx, arm.arm_data, arm.pat.data().unwrap().span, catchall)
}
if !arm.has_guard && catchall.is_none() && pat_is_catchall(arm.pat) {
catchall = Some(arm.pat.data().unwrap().span);
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/or-patterns/exhaustiveness-unreachable-pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,23 @@ fn main() {
| (y, x) => {} //~ ERROR unreachable
}
}

fn unreachable_in_param((_ | (_, _)): (bool, bool)) {}
//~^ ERROR unreachable

fn unreachable_in_binding() {
let bool_pair = (true, true);
let bool_option = Some(true);

let (_ | (_, _)) = bool_pair;
//~^ ERROR unreachable
for (_ | (_, _)) in [bool_pair] {}
//~^ ERROR unreachable

let (Some(_) | Some(true)) = bool_option else { return };
//~^ ERROR unreachable
if let Some(_) | Some(true) = bool_option {}
//~^ ERROR unreachable
while let Some(_) | Some(true) = bool_option {}
//~^ ERROR unreachable
}
38 changes: 37 additions & 1 deletion tests/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -184,5 +184,41 @@ error: unreachable pattern
LL | | (y, x) => {}
| ^^^^^^

error: aborting due to 29 previous errors
error: unreachable pattern
--> $DIR/exhaustiveness-unreachable-pattern.rs:164:30
|
LL | fn unreachable_in_param((_ | (_, _)): (bool, bool)) {}
| ^^^^^^

error: unreachable pattern
--> $DIR/exhaustiveness-unreachable-pattern.rs:171:14
|
LL | let (_ | (_, _)) = bool_pair;
| ^^^^^^

error: unreachable pattern
--> $DIR/exhaustiveness-unreachable-pattern.rs:173:14
|
LL | for (_ | (_, _)) in [bool_pair] {}
| ^^^^^^

error: unreachable pattern
--> $DIR/exhaustiveness-unreachable-pattern.rs:176:20
|
LL | let (Some(_) | Some(true)) = bool_option else { return };
| ^^^^^^^^^^

error: unreachable pattern
--> $DIR/exhaustiveness-unreachable-pattern.rs:178:22
|
LL | if let Some(_) | Some(true) = bool_option {}
| ^^^^^^^^^^

error: unreachable pattern
--> $DIR/exhaustiveness-unreachable-pattern.rs:180:25
|
LL | while let Some(_) | Some(true) = bool_option {}
| ^^^^^^^^^^

error: aborting due to 35 previous errors

44 changes: 44 additions & 0 deletions tests/ui/rfcs/rfc-0000-never_patterns/unreachable.exh_pats.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
error: unreachable pattern
--> $DIR/unreachable.rs:17:9
|
LL | Err(!),
| ^^^^^^
|
note: the lint level is defined here
--> $DIR/unreachable.rs:7:9
|
LL | #![deny(unreachable_patterns)]
| ^^^^^^^^^^^^^^^^^^^^

error: unreachable pattern
--> $DIR/unreachable.rs:20:19
|
LL | let (Ok(_x) | Err(!)) = res_void;
| ^^^^^^

error: unreachable pattern
--> $DIR/unreachable.rs:22:12
|
LL | if let Err(!) = res_void {}
| ^^^^^^

error: unreachable pattern
--> $DIR/unreachable.rs:24:24
|
LL | if let (Ok(true) | Err(!)) = res_void {}
| ^^^^^^

error: unreachable pattern
--> $DIR/unreachable.rs:26:23
|
LL | for (Ok(mut _x) | Err(!)) in [res_void] {}
| ^^^^^^

error: unreachable pattern
--> $DIR/unreachable.rs:30:18
|
LL | fn foo((Ok(_x) | Err(!)): Result<bool, Void>) {}
| ^^^^^^

error: aborting due to 6 previous errors

31 changes: 31 additions & 0 deletions tests/ui/rfcs/rfc-0000-never_patterns/unreachable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// revisions: normal exh_pats
//[normal] check-pass
#![feature(never_patterns)]
#![allow(incomplete_features)]
#![cfg_attr(exh_pats, feature(exhaustive_patterns))]
#![allow(dead_code, unreachable_code)]
#![deny(unreachable_patterns)]

#[derive(Copy, Clone)]
enum Void {}

fn main() {
let res_void: Result<bool, Void> = Ok(true);

match res_void {
Ok(_x) => {}
Err(!),
//[exh_pats]~^ ERROR unreachable
}
let (Ok(_x) | Err(!)) = res_void;
//[exh_pats]~^ ERROR unreachable
if let Err(!) = res_void {}
//[exh_pats]~^ ERROR unreachable
if let (Ok(true) | Err(!)) = res_void {}
//[exh_pats]~^ ERROR unreachable
for (Ok(mut _x) | Err(!)) in [res_void] {}
//[exh_pats]~^ ERROR unreachable
}

fn foo((Ok(_x) | Err(!)): Result<bool, Void>) {}
//[exh_pats]~^ ERROR unreachable
1 change: 1 addition & 0 deletions tests/ui/unsafe/union_destructure.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// run-pass
#![allow(unreachable_patterns)]

#[derive(Copy, Clone)]
#[allow(dead_code)]
Expand Down

0 comments on commit 64cea92

Please sign in to comment.