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

Rename overlapping_patterns lint #78242

Merged
merged 6 commits into from
Dec 22, 2020
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
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
UNUSED_MUT,
UNREACHABLE_CODE,
UNREACHABLE_PATTERNS,
OVERLAPPING_PATTERNS,
UNUSED_MUST_USE,
UNUSED_UNSAFE,
PATH_STATEMENTS,
Expand Down Expand Up @@ -335,6 +334,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
store.register_renamed("exceeding_bitshifts", "arithmetic_overflow");
store.register_renamed("redundant_semicolon", "redundant_semicolons");
store.register_renamed("intra_doc_link_resolution_failure", "broken_intra_doc_links");
store.register_renamed("overlapping_patterns", "overlapping_range_endpoints");
store.register_removed("unknown_features", "replaced by an error");
store.register_removed("unsigned_negation", "replaced by negate_unsigned feature gate");
store.register_removed("negate_unsigned", "cast a signed value instead");
Expand Down
17 changes: 8 additions & 9 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,8 @@ declare_lint! {
}

declare_lint! {
/// The `overlapping_patterns` lint detects `match` arms that have
/// [range patterns] that overlap.
/// The `overlapping_range_endpoints` lint detects `match` arms that have [range patterns] that
/// overlap on their endpoints.
///
/// [range patterns]: https://doc.rust-lang.org/nightly/reference/patterns.html#range-patterns
///
Expand All @@ -607,13 +607,12 @@ declare_lint! {
///
/// ### Explanation
///
/// It is likely a mistake to have range patterns in a match expression
/// that overlap. Check that the beginning and end values are what you
/// expect, and keep in mind that with `..=` the left and right bounds are
/// inclusive.
pub OVERLAPPING_PATTERNS,
/// It is likely a mistake to have range patterns in a match expression that overlap in this
/// way. Check that the beginning and end values are what you expect, and keep in mind that
/// with `..=` the left and right bounds are inclusive.
pub OVERLAPPING_RANGE_ENDPOINTS,
Warn,
"detects overlapping patterns"
"detects range patterns with overlapping endpoints"
}

declare_lint! {
Expand Down Expand Up @@ -2765,7 +2764,7 @@ declare_lint_pass! {
DEAD_CODE,
UNREACHABLE_CODE,
UNREACHABLE_PATTERNS,
OVERLAPPING_PATTERNS,
OVERLAPPING_RANGE_ENDPOINTS,
BINDINGS_WITH_VARIANT_NAME,
UNUSED_MACROS,
WARNINGS,
Expand Down
18 changes: 9 additions & 9 deletions compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ impl IntRange {
// 2 -------- // 2 -------
let (lo, hi) = self.boundaries();
let (other_lo, other_hi) = other.boundaries();
lo == other_hi || hi == other_lo
(lo == other_hi || hi == other_lo) && !self.is_singleton() && !other.is_singleton()
}

fn to_pat<'tcx>(&self, tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Pat<'tcx> {
Expand Down Expand Up @@ -273,7 +273,7 @@ impl IntRange {
let mut borders: Vec<_> = row_borders.chain(self_borders).collect();
borders.sort_unstable();

self.lint_overlapping_patterns(pcx, hir_id, overlaps);
self.lint_overlapping_range_endpoints(pcx, hir_id, overlaps);

// We're going to iterate through every adjacent pair of borders, making sure that
// each represents an interval of nonnegative length, and convert each such
Expand All @@ -296,30 +296,30 @@ impl IntRange {
.collect()
}

fn lint_overlapping_patterns(
fn lint_overlapping_range_endpoints(
&self,
pcx: PatCtxt<'_, '_, '_>,
hir_id: Option<HirId>,
overlaps: Vec<(IntRange, Span)>,
) {
if let (true, Some(hir_id)) = (!overlaps.is_empty(), hir_id) {
pcx.cx.tcx.struct_span_lint_hir(
lint::builtin::OVERLAPPING_PATTERNS,
lint::builtin::OVERLAPPING_RANGE_ENDPOINTS,
hir_id,
pcx.span,
|lint| {
let mut err = lint.build("multiple patterns covering the same range");
err.span_label(pcx.span, "overlapping patterns");
let mut err = lint.build("multiple patterns overlap on their endpoints");
for (int_range, span) in overlaps {
// Use the real type for user display of the ranges:
err.span_label(
span,
&format!(
"this range overlaps on `{}`",
int_range.to_pat(pcx.cx.tcx, pcx.ty),
"this range overlaps on `{}`...",
int_range.to_pat(pcx.cx.tcx, pcx.ty)
),
);
}
err.span_label(pcx.span, "... with this range");
Copy link
Member

Choose a reason for hiding this comment

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

Ah, this didn't have the effect I hoped it would. I forget how vertical label ordering is determined: I guess by span location?

Copy link
Member Author

@Nadrieril Nadrieril Dec 20, 2020

Choose a reason for hiding this comment

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

I'm not sure, but I think it's only the tests that involve macros that are confusing. In normal cases the ranges will be on different lines of a match and the messages will be in the order we want them

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's true. Not worth worrying about.

err.note("you likely meant to write mutually exclusive ranges");
err.emit();
},
);
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-21475.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// run-pass
#![allow(unused_imports, overlapping_patterns)]
#![allow(unused_imports, overlapping_range_endpoints)]
// pretty-expanded FIXME #23616

use m::{START, END};
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-26251.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// run-pass
#![allow(overlapping_patterns)]
#![allow(overlapping_range_endpoints)]

fn main() {
let x = 'a';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![feature(exclusive_range_pattern)]
#![feature(assoc_char_consts)]
#![allow(overlapping_range_endpoints)]
#![deny(unreachable_patterns)]

macro_rules! m {
Expand Down
24 changes: 12 additions & 12 deletions src/test/ui/pattern/usefulness/integer-ranges/exhaustiveness.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0004]: non-exhaustive patterns: `u8::MAX` not covered
--> $DIR/exhaustiveness.rs:47:8
--> $DIR/exhaustiveness.rs:48:8
|
LL | m!(0u8, 0..255);
| ^^^ pattern `u8::MAX` not covered
Expand All @@ -8,7 +8,7 @@ LL | m!(0u8, 0..255);
= note: the matched value is of type `u8`

error[E0004]: non-exhaustive patterns: `u8::MAX` not covered
--> $DIR/exhaustiveness.rs:48:8
--> $DIR/exhaustiveness.rs:49:8
|
LL | m!(0u8, 0..=254);
| ^^^ pattern `u8::MAX` not covered
Expand All @@ -17,7 +17,7 @@ LL | m!(0u8, 0..=254);
= note: the matched value is of type `u8`

error[E0004]: non-exhaustive patterns: `0_u8` not covered
--> $DIR/exhaustiveness.rs:49:8
--> $DIR/exhaustiveness.rs:50:8
|
LL | m!(0u8, 1..=255);
| ^^^ pattern `0_u8` not covered
Expand All @@ -26,7 +26,7 @@ LL | m!(0u8, 1..=255);
= note: the matched value is of type `u8`

error[E0004]: non-exhaustive patterns: `42_u8` not covered
--> $DIR/exhaustiveness.rs:50:8
--> $DIR/exhaustiveness.rs:51:8
|
LL | m!(0u8, 0..42 | 43..=255);
| ^^^ pattern `42_u8` not covered
Expand All @@ -35,7 +35,7 @@ LL | m!(0u8, 0..42 | 43..=255);
= note: the matched value is of type `u8`

error[E0004]: non-exhaustive patterns: `i8::MAX` not covered
--> $DIR/exhaustiveness.rs:51:8
--> $DIR/exhaustiveness.rs:52:8
|
LL | m!(0i8, -128..127);
| ^^^ pattern `i8::MAX` not covered
Expand All @@ -44,7 +44,7 @@ LL | m!(0i8, -128..127);
= note: the matched value is of type `i8`

error[E0004]: non-exhaustive patterns: `i8::MAX` not covered
--> $DIR/exhaustiveness.rs:52:8
--> $DIR/exhaustiveness.rs:53:8
|
LL | m!(0i8, -128..=126);
| ^^^ pattern `i8::MAX` not covered
Expand All @@ -53,7 +53,7 @@ LL | m!(0i8, -128..=126);
= note: the matched value is of type `i8`

error[E0004]: non-exhaustive patterns: `i8::MIN` not covered
--> $DIR/exhaustiveness.rs:53:8
--> $DIR/exhaustiveness.rs:54:8
|
LL | m!(0i8, -127..=127);
| ^^^ pattern `i8::MIN` not covered
Expand All @@ -62,7 +62,7 @@ LL | m!(0i8, -127..=127);
= note: the matched value is of type `i8`

error[E0004]: non-exhaustive patterns: `0_i8` not covered
--> $DIR/exhaustiveness.rs:54:11
--> $DIR/exhaustiveness.rs:55:11
|
LL | match 0i8 {
| ^^^ pattern `0_i8` not covered
Expand All @@ -71,7 +71,7 @@ LL | match 0i8 {
= note: the matched value is of type `i8`

error[E0004]: non-exhaustive patterns: `u128::MAX` not covered
--> $DIR/exhaustiveness.rs:59:8
--> $DIR/exhaustiveness.rs:60:8
|
LL | m!(0u128, 0..=ALMOST_MAX);
| ^^^^^ pattern `u128::MAX` not covered
Expand All @@ -80,7 +80,7 @@ LL | m!(0u128, 0..=ALMOST_MAX);
= note: the matched value is of type `u128`

error[E0004]: non-exhaustive patterns: `5_u128..=u128::MAX` not covered
--> $DIR/exhaustiveness.rs:60:8
--> $DIR/exhaustiveness.rs:61:8
|
LL | m!(0u128, 0..=4);
| ^^^^^ pattern `5_u128..=u128::MAX` not covered
Expand All @@ -89,7 +89,7 @@ LL | m!(0u128, 0..=4);
= note: the matched value is of type `u128`

error[E0004]: non-exhaustive patterns: `0_u128` not covered
--> $DIR/exhaustiveness.rs:61:8
--> $DIR/exhaustiveness.rs:62:8
|
LL | m!(0u128, 1..=u128::MAX);
| ^^^^^ pattern `0_u128` not covered
Expand All @@ -98,7 +98,7 @@ LL | m!(0u128, 1..=u128::MAX);
= note: the matched value is of type `u128`

error[E0004]: non-exhaustive patterns: `(126_u8..=127_u8, false)` not covered
--> $DIR/exhaustiveness.rs:69:11
--> $DIR/exhaustiveness.rs:70:11
|
LL | match (0u8, true) {
| ^^^^^^^^^^^ pattern `(126_u8..=127_u8, false)` not covered
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![feature(exclusive_range_pattern)]
#![deny(overlapping_patterns)]
#![deny(overlapping_range_endpoints)]

macro_rules! m {
($s:expr, $t1:pat, $t2:pat) => {
Expand All @@ -12,27 +12,33 @@ macro_rules! m {
}

fn main() {
m!(0u8, 20..=30, 30..=40); //~ ERROR multiple patterns covering the same range
m!(0u8, 30..=40, 20..=30); //~ ERROR multiple patterns covering the same range
m!(0u8, 20..=30, 30..=40); //~ ERROR multiple patterns overlap on their endpoints
m!(0u8, 30..=40, 20..=30); //~ ERROR multiple patterns overlap on their endpoints
m!(0u8, 20..=30, 31..=40);
m!(0u8, 20..=30, 29..=40);
m!(0u8, 20.. 30, 29..=40); //~ ERROR multiple patterns covering the same range
m!(0u8, 20.. 30, 29..=40); //~ ERROR multiple patterns overlap on their endpoints
m!(0u8, 20.. 30, 28..=40);
m!(0u8, 20.. 30, 30..=40);
m!(0u8, 20..=30, 30..=30);
m!(0u8, 20..=30, 30..=31); //~ ERROR multiple patterns covering the same range
m!(0u8, 20..=30, 30..=31); //~ ERROR multiple patterns overlap on their endpoints
m!(0u8, 20..=30, 29..=30);
m!(0u8, 20..=30, 20..=20);
m!(0u8, 20..=30, 20..=21);
m!(0u8, 20..=30, 19..=20); //~ ERROR multiple patterns covering the same range
m!(0u8, 20..=30, 19..=20); //~ ERROR multiple patterns overlap on their endpoints
m!(0u8, 20..=30, 20);
m!(0u8, 20..=30, 25);
m!(0u8, 20..=30, 30);
m!(0u8, 20.. 30, 29);
m!(0u8, 20, 20..=30); //~ ERROR multiple patterns covering the same range
m!(0u8, 20, 20..=30);
m!(0u8, 25, 20..=30);
m!(0u8, 30, 20..=30); //~ ERROR multiple patterns covering the same range
m!(0u8, 30, 20..=30);

match 0u8 {
0..=10 => {}
20..=30 => {}
10..=20 => {} //~ ERROR multiple patterns overlap on their endpoints
_ => {}
}
match (0u8, true) {
(0..=10, true) => {}
(10..20, true) => {} // not detected
Expand All @@ -41,13 +47,13 @@ fn main() {
}
match (true, 0u8) {
(true, 0..=10) => {}
(true, 10..20) => {} //~ ERROR multiple patterns covering the same range
(true, 10..20) => {} //~ ERROR multiple patterns overlap on their endpoints
(false, 10..20) => {}
_ => {}
}
match Some(0u8) {
Some(0..=10) => {}
Some(10..20) => {} //~ ERROR multiple patterns covering the same range
Some(10..20) => {} //~ ERROR multiple patterns overlap on their endpoints
_ => {}
}
}
Loading