Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report unreachable subpatterns consistently #120097

Merged
merged 3 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -386,6 +385,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 @@ -431,14 +458,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 @@ -470,7 +490,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 @@ -552,7 +572,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 @@ -563,7 +583,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 @@ -833,39 +852,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(!),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, just double checking my understanding -- with exhaustive patterns, we don't need to care about never patterns b/c the scrutinee already disqualifies those patterns?

Copy link
Member Author

@Nadrieril Nadrieril Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Said differently, never patterns are shorthand syntax: Err(!) is short for Err(_) => unreachable_unchecked!(). The rules for which arms are required are orthogonal, and here exhaustive_patterns says "we don't need that arm". I've updated the tracking issue to clarify that while I was at it

//[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