From 12ebc3dd966e1accc7d8e73aa4a4f3ea8d4bd9eb Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 18 Jan 2024 15:06:46 +0100 Subject: [PATCH 1/3] Add tests --- .../exhaustiveness-unreachable-pattern.rs | 18 +++++++++++ .../exhaustiveness-unreachable-pattern.stderr | 26 ++++++++++++++- .../unreachable.exh_pats.stderr | 32 +++++++++++++++++++ .../rfc-0000-never_patterns/unreachable.rs | 29 +++++++++++++++++ 4 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 tests/ui/rfcs/rfc-0000-never_patterns/unreachable.exh_pats.stderr create mode 100644 tests/ui/rfcs/rfc-0000-never_patterns/unreachable.rs diff --git a/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.rs b/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.rs index 20a8d754996..324ba54e0e7 100644 --- a/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.rs +++ b/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.rs @@ -160,3 +160,21 @@ fn main() { | (y, x) => {} //~ ERROR unreachable } } + +fn unreachable_in_param((_ | (_, _)): (bool, bool)) {} + +fn unreachable_in_binding() { + let bool_pair = (true, true); + let bool_option = Some(true); + + let (_ | (_, _)) = bool_pair; + 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 +} diff --git a/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr b/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr index 3616dda9981..d47645a4689 100644 --- a/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr +++ b/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr @@ -184,5 +184,29 @@ error: unreachable pattern LL | | (y, x) => {} | ^^^^^^ -error: aborting due to 29 previous errors +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:171:14 + | +LL | for (_ | (_, _)) in [bool_pair] {} + | ^^^^^^ + +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:174:20 + | +LL | let (Some(_) | Some(true)) = bool_option else { return }; + | ^^^^^^^^^^ + +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:176:22 + | +LL | if let Some(_) | Some(true) = bool_option {} + | ^^^^^^^^^^ + +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:178:25 + | +LL | while let Some(_) | Some(true) = bool_option {} + | ^^^^^^^^^^ + +error: aborting due to 33 previous errors diff --git a/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.exh_pats.stderr b/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.exh_pats.stderr new file mode 100644 index 00000000000..5a1d9ca50bd --- /dev/null +++ b/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.exh_pats.stderr @@ -0,0 +1,32 @@ +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:21:12 + | +LL | if let Err(!) = res_void {} + | ^^^^^^ + +error: unreachable pattern + --> $DIR/unreachable.rs:23:24 + | +LL | if let (Ok(true) | Err(!)) = res_void {} + | ^^^^^^ + +error: unreachable pattern + --> $DIR/unreachable.rs:25:23 + | +LL | for (Ok(mut _x) | Err(!)) in [res_void] {} + | ^^^^^^ + +error: aborting due to 4 previous errors + diff --git a/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.rs b/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.rs new file mode 100644 index 00000000000..7bc695fd962 --- /dev/null +++ b/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.rs @@ -0,0 +1,29 @@ +// 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 = Ok(true); + + match res_void { + Ok(_x) => {} + Err(!), + //[exh_pats]~^ ERROR unreachable + } + let (Ok(_x) | Err(!)) = res_void; + 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) {} From 753680afe833ada2ffe4723519d127dc2d256e78 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 18 Jan 2024 14:45:41 +0100 Subject: [PATCH 2/3] Consistently set `MatchVisitor.error` on error --- .../src/thir/pattern/check_match.rs | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index f6c5e4a5cd6..b2cb4240cf5 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -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::*; @@ -386,6 +385,18 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { } } + fn analyze_patterns( + &mut self, + cx: &MatchCheckCtxt<'p, 'tcx>, + arms: &[MatchArm<'p, 'tcx>], + scrut_ty: Ty<'tcx>, + ) -> Result, ErrorGuaranteed> { + rustc_pattern_analysis::analyze_match(&cx, &arms, scrut_ty).map_err(|err| { + self.error = Err(err); + err + }) + } + #[instrument(level = "trace", skip(self))] fn check_let(&mut self, pat: &'p Pat<'tcx>, scrutinee: Option, span: Span) { assert!(self.let_source != LetSource::None); @@ -431,14 +442,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 => .. }` @@ -470,7 +474,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, )); } } @@ -552,7 +556,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)) } From 0a9bb9722907cbbd75a643fad1af3278517805fc Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 18 Jan 2024 14:59:48 +0100 Subject: [PATCH 3/3] Consistently warn unreachable subpatterns --- .../src/thir/pattern/check_match.rs | 68 ++++++++++--------- .../exhaustiveness-unreachable-pattern.rs | 2 + .../exhaustiveness-unreachable-pattern.stderr | 20 ++++-- .../unreachable.exh_pats.stderr | 20 ++++-- .../rfc-0000-never_patterns/unreachable.rs | 2 + tests/ui/unsafe/union_destructure.rs | 1 + 6 files changed, 74 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index b2cb4240cf5..dbd719db559 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -391,10 +391,26 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { arms: &[MatchArm<'p, 'tcx>], scrut_ty: Ty<'tcx>, ) -> Result, ErrorGuaranteed> { - rustc_pattern_analysis::analyze_match(&cx, &arms, scrut_ty).map_err(|err| { - self.error = Err(err); - err - }) + 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))] @@ -567,7 +583,6 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { ) -> Result { 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. @@ -837,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, +) { + 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| { - 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); diff --git a/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.rs b/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.rs index 324ba54e0e7..1ad335bf394 100644 --- a/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.rs +++ b/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.rs @@ -162,12 +162,14 @@ fn main() { } 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 diff --git a/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr b/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr index d47645a4689..336530d1b32 100644 --- a/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr +++ b/tests/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr @@ -184,29 +184,41 @@ error: unreachable pattern LL | | (y, x) => {} | ^^^^^^ +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:174:20 + --> $DIR/exhaustiveness-unreachable-pattern.rs:176:20 | LL | let (Some(_) | Some(true)) = bool_option else { return }; | ^^^^^^^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:176:22 + --> $DIR/exhaustiveness-unreachable-pattern.rs:178:22 | LL | if let Some(_) | Some(true) = bool_option {} | ^^^^^^^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:178:25 + --> $DIR/exhaustiveness-unreachable-pattern.rs:180:25 | LL | while let Some(_) | Some(true) = bool_option {} | ^^^^^^^^^^ -error: aborting due to 33 previous errors +error: aborting due to 35 previous errors diff --git a/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.exh_pats.stderr b/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.exh_pats.stderr index 5a1d9ca50bd..fe2a72d2a31 100644 --- a/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.exh_pats.stderr +++ b/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.exh_pats.stderr @@ -11,22 +11,34 @@ LL | #![deny(unreachable_patterns)] | ^^^^^^^^^^^^^^^^^^^^ error: unreachable pattern - --> $DIR/unreachable.rs:21:12 + --> $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:23:24 + --> $DIR/unreachable.rs:24:24 | LL | if let (Ok(true) | Err(!)) = res_void {} | ^^^^^^ error: unreachable pattern - --> $DIR/unreachable.rs:25:23 + --> $DIR/unreachable.rs:26:23 | LL | for (Ok(mut _x) | Err(!)) in [res_void] {} | ^^^^^^ -error: aborting due to 4 previous errors +error: unreachable pattern + --> $DIR/unreachable.rs:30:18 + | +LL | fn foo((Ok(_x) | Err(!)): Result) {} + | ^^^^^^ + +error: aborting due to 6 previous errors diff --git a/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.rs b/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.rs index 7bc695fd962..df8e22abf62 100644 --- a/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.rs +++ b/tests/ui/rfcs/rfc-0000-never_patterns/unreachable.rs @@ -18,6 +18,7 @@ fn main() { //[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 {} @@ -27,3 +28,4 @@ fn main() { } fn foo((Ok(_x) | Err(!)): Result) {} +//[exh_pats]~^ ERROR unreachable diff --git a/tests/ui/unsafe/union_destructure.rs b/tests/ui/unsafe/union_destructure.rs index d0cf8640eaa..bb75dbf0997 100644 --- a/tests/ui/unsafe/union_destructure.rs +++ b/tests/ui/unsafe/union_destructure.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unreachable_patterns)] #[derive(Copy, Clone)] #[allow(dead_code)]