nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-25 00:03:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Lint small gaps between ranges

Browse Source
This commit is contained in:
Nadrieril 2024-01-14 22:24:10 +01:00
parent f783043ebf
commit 8ac9a04257
7 changed files with 492 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
compiler/rustc_pattern_analysis/messages.ftl
View File

@ -1,3 +1,11 @@
pattern_analysis_excluside_range_missing_gap = multiple ranges are one apart
.label = this range doesn't match `{$gap}` because `..` is an exclusive range
.suggestion = use an inclusive range instead
pattern_analysis_excluside_range_missing_max = exclusive range missing `{$max}`
.label = this range doesn't match `{$max}` because `..` is an exclusive range
.suggestion = use an inclusive range instead
pattern_analysis_non_exhaustive_omitted_pattern = some variants are not matched explicitly
.help = ensure that all variants are matched explicitly by adding the suggested match arms
.note = the matched value is of type `{$scrut_ty}` and the `non_exhaustive_omitted_patterns` attribute was found

51
compiler/rustc_pattern_analysis/src/errors.rs
View File

@ -76,6 +76,57 @@ impl<'tcx> AddToDiagnostic for Overlap<'tcx> {
}
}
#[derive(LintDiagnostic)]
#[diag(pattern_analysis_excluside_range_missing_max)]
pub struct ExclusiveRangeMissingMax<'tcx> {
#[label]
#[suggestion(code = "{suggestion}", applicability = "maybe-incorrect")]
/// This is an exclusive range that looks like `lo..max` (i.e. doesn't match `max`).
pub first_range: Span,
/// Suggest `lo..=max` instead.
pub suggestion: String,
pub max: Pat<'tcx>,
}
#[derive(LintDiagnostic)]
#[diag(pattern_analysis_excluside_range_missing_gap)]
pub struct ExclusiveRangeMissingGap<'tcx> {
#[label]
#[suggestion(code = "{suggestion}", applicability = "maybe-incorrect")]
/// This is an exclusive range that looks like `lo..gap` (i.e. doesn't match `gap`).
pub first_range: Span,
pub gap: Pat<'tcx>,
/// Suggest `lo..=gap` instead.
pub suggestion: String,
#[subdiagnostic]
/// All these ranges skipped over `gap` which we think is probably a mistake.
pub gap_with: Vec<GappedRange<'tcx>>,
}
pub struct GappedRange<'tcx> {
pub span: Span,
pub gap: Pat<'tcx>,
pub first_range: Pat<'tcx>,
}
impl<'tcx> AddToDiagnostic for GappedRange<'tcx> {
fn add_to_diagnostic_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
self,
diag: &mut Diag<'_, G>,
_: F,
) {
let GappedRange { span, gap, first_range } = self;
// FIXME(mejrs) unfortunately `#[derive(LintDiagnostic)]`
// does not support `#[subdiagnostic(eager)]`...
let message = format!(
"this could appear to continue range `{first_range}`, but `{gap}` isn't matched by \
either of them"
);
diag.span_label(span, message);
}
}
#[derive(LintDiagnostic)]
#[diag(pattern_analysis_non_exhaustive_omitted_pattern)]
#[help]

23
compiler/rustc_pattern_analysis/src/lib.rs
View File

@ -70,14 +70,8 @@ use rustc_middle::ty::Ty;
use rustc_span::ErrorGuaranteed;
use crate::constructor::{Constructor, ConstructorSet, IntRange};
#[cfg(feature = "rustc")]
use crate::lints::lint_nonexhaustive_missing_variants;
use crate::pat::DeconstructedPat;
use crate::pat_column::PatternColumn;
#[cfg(feature = "rustc")]
use crate::rustc::RustcMatchCheckCtxt;
#[cfg(feature = "rustc")]
use crate::usefulness::{compute_match_usefulness, ValidityConstraint};
pub trait Captures<'a> {}
impl<'a, T: ?Sized> Captures<'a> for T {}
@ -145,6 +139,18 @@ pub trait TypeCx: Sized + fmt::Debug {
/// The maximum pattern complexity limit was reached.
fn complexity_exceeded(&self) -> Result<(), Self::Error>;
/// Lint that there is a gap `gap` between `pat` and all of `gapped_with` such that the gap is
/// not matched by another range. If `gapped_with` is empty, then `gap` is `T::MAX`. We only
/// detect singleton gaps.
/// The default implementation does nothing.
fn lint_non_contiguous_range_endpoints(
&self,
_pat: &DeconstructedPat<Self>,
_gap: IntRange,
_gapped_with: &[&DeconstructedPat<Self>],
) {
}
}
/// The arm of a match expression.
@ -167,11 +173,14 @@ impl<'p, Cx: TypeCx> Copy for MatchArm<'p, Cx> {}
/// useful, and runs some lints.
#[cfg(feature = "rustc")]
pub fn analyze_match<'p, 'tcx>(
tycx: &RustcMatchCheckCtxt<'p, 'tcx>,
tycx: &rustc::RustcMatchCheckCtxt<'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, ValidityConstraint};
let scrut_ty = tycx.reveal_opaque_ty(scrut_ty);
let scrut_validity = ValidityConstraint::from_bool(tycx.known_valid_scrutinee);
let report =

66
compiler/rustc_pattern_analysis/src/rustc.rs
View File

@ -8,7 +8,7 @@ use rustc_index::{Idx, IndexVec};
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::{self, Const};
use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange, PatRangeBoundary};
use rustc_middle::thir::{self, FieldPat, Pat, PatKind, PatRange, PatRangeBoundary};
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::{self, FieldDef, OpaqueTypeKey, Ty, TyCtxt, TypeVisitableExt, VariantDef};
use rustc_session::lint;
@ -900,6 +900,70 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> {
let span = self.whole_match_span.unwrap_or(self.scrut_span);
Err(self.tcx.dcx().span_err(span, "reached pattern complexity limit"))
}
fn lint_non_contiguous_range_endpoints(
&self,
pat: &crate::pat::DeconstructedPat<Self>,
gap: IntRange,
gapped_with: &[&crate::pat::DeconstructedPat<Self>],
) {
let Some(&thir_pat) = pat.data() else { return };
let thir::PatKind::Range(range) = &thir_pat.kind else { return };
// Only lint when the left range is an exclusive range.
if range.end != rustc_hir::RangeEnd::Excluded {
return;
}
// `pat` is an exclusive range like `lo..gap`. `gapped_with` contains ranges that start with
// `gap+1`.
let suggested_range: thir::Pat<'_> = {
// Suggest `lo..=gap` instead.
let mut suggested_range = thir_pat.clone();
let thir::PatKind::Range(range) = &mut suggested_range.kind else { unreachable!() };
range.end = rustc_hir::RangeEnd::Included;
suggested_range
};
let gap_as_pat = self.hoist_pat_range(&gap, *pat.ty());
if gapped_with.is_empty() {
// If `gapped_with` is empty, `gap == T::MAX`.
self.tcx.emit_node_span_lint(
lint::builtin::NON_CONTIGUOUS_RANGE_ENDPOINTS,
self.match_lint_level,
thir_pat.span,
errors::ExclusiveRangeMissingMax {
// Point at this range.
first_range: thir_pat.span,
// That's the gap that isn't covered.
max: gap_as_pat.clone(),
// Suggest `lo..=max` instead.
suggestion: suggested_range.to_string(),
},
);
} else {
self.tcx.emit_node_span_lint(
lint::builtin::NON_CONTIGUOUS_RANGE_ENDPOINTS,
self.match_lint_level,
thir_pat.span,
errors::ExclusiveRangeMissingGap {
// Point at this range.
first_range: thir_pat.span,
// That's the gap that isn't covered.
gap: gap_as_pat.clone(),
// Suggest `lo..=gap` instead.
suggestion: suggested_range.to_string(),
// All these ranges skipped over `gap` which we think is probably a
// mistake.
gap_with: gapped_with
.iter()
.map(|pat| errors::GappedRange {
span: pat.data().unwrap().span,
gap: gap_as_pat.clone(),
first_range: thir_pat.clone(),
})
.collect(),
},
);
}
}
}
/// Recursively expand this pattern into its subpatterns. Only useful for or-patterns.

46
compiler/rustc_pattern_analysis/src/usefulness.rs
View File

@ -1489,7 +1489,7 @@ impl<Cx: TypeCx> WitnessMatrix<Cx> {
/// We can however get false negatives because exhaustiveness does not explore all cases. See the
/// section on relevancy at the top of the file.
fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
mcx: &mut UsefulnessCtxt<'_, Cx>,
cx: &Cx,
overlap_range: IntRange,
matrix: &Matrix<'p, Cx>,
specialized_matrix: &Matrix<'p, Cx>,
@ -1522,7 +1522,7 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
.map(|&(_, pat)| pat)
.collect();
if !overlaps_with.is_empty() {
mcx.tycx.lint_overlapping_range_endpoints(pat, overlap_range, &overlaps_with);
cx.lint_overlapping_range_endpoints(pat, overlap_range, &overlaps_with);
}
}
suffixes.push((child_row_id, pat))
@ -1538,7 +1538,7 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
.map(|&(_, pat)| pat)
.collect();
if !overlaps_with.is_empty() {
mcx.tycx.lint_overlapping_range_endpoints(pat, overlap_range, &overlaps_with);
cx.lint_overlapping_range_endpoints(pat, overlap_range, &overlaps_with);
}
}
prefixes.push((child_row_id, pat))
@ -1546,6 +1546,33 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
}
}
/// Collect ranges that have a singleton gap between them.
fn collect_non_contiguous_range_endpoints<'p, Cx: TypeCx>(
cx: &Cx,
gap_range: &IntRange,
matrix: &Matrix<'p, Cx>,
) {
let gap = gap_range.lo;
// Ranges that look like `lo..gap`.
let mut onebefore: SmallVec<[_; 1]> = Default::default();
// Ranges that start on `gap+1` or singletons `gap+1`.
let mut oneafter: SmallVec<[_; 1]> = Default::default();
// Look through the column for ranges near the gap.
for pat in matrix.heads() {
let PatOrWild::Pat(pat) = pat else { continue };
let Constructor::IntRange(this_range) = pat.ctor() else { continue };
if gap == this_range.hi {
onebefore.push(pat)
} else if gap.plus_one() == Some(this_range.lo) {
oneafter.push(pat)
}
}
for pat_before in onebefore {
cx.lint_non_contiguous_range_endpoints(pat_before, *gap_range, oneafter.as_slice());
}
}
/// The core of the algorithm.
///
/// This recursively computes witnesses of the non-exhaustiveness of `matrix` (if any). Also tracks
@ -1626,13 +1653,24 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
&& spec_matrix.rows.len() >= 2
&& spec_matrix.rows.iter().any(|row| !row.intersects.is_empty())
{
collect_overlapping_range_endpoints(mcx, overlap_range, matrix, &spec_matrix);
collect_overlapping_range_endpoints(mcx.tycx, overlap_range, matrix, &spec_matrix);
}
}
matrix.unspecialize(spec_matrix);
}
// Detect singleton gaps between ranges.
if missing_ctors.iter().any(|c| matches!(c, Constructor::IntRange(..))) {
for missing in &missing_ctors {
if let Constructor::IntRange(gap) = missing {
if gap.is_singleton() {
collect_non_contiguous_range_endpoints(mcx.tycx, gap, matrix);
}
}
}
}
// Record usefulness in the patterns.
for row in matrix.rows() {
if row.useful {

117
tests/ui/pattern/usefulness/integer-ranges/gap_between_ranges.rs Normal file
View File

@ -0,0 +1,117 @@
#![feature(exclusive_range_pattern)]
#![deny(non_contiguous_range_endpoints)]
macro_rules! m {
($s:expr, $t1:pat, $t2:pat) => {
match $s {
$t1 => {}
$t2 => {}
_ => {}
}
};
}
fn main() {
match 0u8 {
20..30 => {} //~ ERROR multiple ranges are one apart
31..=40 => {}
_ => {}
}
match 0u8 {
20..30 => {} //~ ERROR multiple ranges are one apart
31 => {}
_ => {}
}
m!(0u8, 20..30, 31..=40); //~ ERROR multiple ranges are one apart
m!(0u8, 31..=40, 20..30); //~ ERROR multiple ranges are one apart
m!(0u8, 20..30, 29..=40); //~ WARN multiple patterns overlap on their endpoints
m!(0u8, 20..30, 30..=40);
m!(0u8, 20..30, 31..=40); //~ ERROR multiple ranges are one apart
m!(0u8, 20..30, 32..=40);
m!(0u8, 20..30, 31..=32); //~ ERROR multiple ranges are one apart
// Don't lint two singletons.
m!(0u8, 30, 32);
// Don't lint on the equivalent inclusive range
m!(0u8, 20..=29, 31..=40);
// Don't lint if the lower one is a singleton.
m!(0u8, 30, 32..=40);
// Catch the `lo..MAX` case.
match 0u8 {
0..255 => {} //~ ERROR exclusive range missing `u8::MAX`
_ => {}
}
// Except for `usize`, since `0..=usize::MAX` isn't exhaustive either.
match 0usize {
0..usize::MAX => {}
_ => {}
}
// Don't lint if the gap is caught by another range.
match 0u8 {
0..10 => {}
11..20 => {}
10 => {}
_ => {}
}
match 0u8 {
0..10 => {}
11..20 => {}
5..15 => {}
_ => {}
}
match 0u8 {
0..255 => {}
255 => {}
}
// Test multiple at once
match 0u8 {
0..10 => {} //~ ERROR multiple ranges are one apart
11..20 => {}
11..30 => {}
_ => {}
}
match 0u8 {
0..10 => {} //~ ERROR multiple ranges are one apart
11..20 => {} //~ ERROR multiple ranges are one apart
21..30 => {}
_ => {}
}
match 0u8 {
00..20 => {} //~ ERROR multiple ranges are one apart
10..20 => {} //~ ERROR multiple ranges are one apart
21..30 => {}
21..40 => {}
_ => {}
}
// Test nested
match (0u8, true) {
(0..10, true) => {} //~ ERROR multiple ranges are one apart
(11..20, true) => {}
_ => {}
}
match (true, 0u8) {
(true, 0..10) => {} //~ ERROR multiple ranges are one apart
(true, 11..20) => {}
_ => {}
}
// Asymmetry due to how exhaustiveness is computed.
match (0u8, true) {
(0..10, true) => {} //~ ERROR multiple ranges are one apart
(11..20, false) => {}
_ => {}
}
match (true, 0u8) {
(true, 0..10) => {}
(false, 11..20) => {}
_ => {}
}
match Some(0u8) {
Some(0..10) => {} //~ ERROR multiple ranges are one apart
Some(11..20) => {}
_ => {}
}
}

193
tests/ui/pattern/usefulness/integer-ranges/gap_between_ranges.stderr Normal file
View File

@ -0,0 +1,193 @@
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:16:9
|
LL | 20..30 => {}
| ^^^^^^
| |
| this range doesn't match `30_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `20_u8..=30_u8`
LL | 31..=40 => {}
| ------- this could appear to continue range `20_u8..30_u8`, but `30_u8` isn't matched by either of them
|
note: the lint level is defined here
--> $DIR/gap_between_ranges.rs:2:9
|
LL | #![deny(non_contiguous_range_endpoints)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:21:9
|
LL | 20..30 => {}
| ^^^^^^
| |
| this range doesn't match `30_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `20_u8..=30_u8`
LL | 31 => {}
| -- this could appear to continue range `20_u8..30_u8`, but `30_u8` isn't matched by either of them
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:26:13
|
LL | m!(0u8, 20..30, 31..=40);
| ^^^^^^ ------- this could appear to continue range `20_u8..30_u8`, but `30_u8` isn't matched by either of them
| |
| this range doesn't match `30_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `20_u8..=30_u8`
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:27:22
|
LL | m!(0u8, 31..=40, 20..30);
| ------- ^^^^^^
| | |
| | this range doesn't match `30_u8` because `..` is an exclusive range
| | help: use an inclusive range instead: `20_u8..=30_u8`
| this could appear to continue range `20_u8..30_u8`, but `30_u8` isn't matched by either of them
warning: multiple patterns overlap on their endpoints
--> $DIR/gap_between_ranges.rs:28:21
|
LL | m!(0u8, 20..30, 29..=40);
| ------ ^^^^^^^ ... with this range
| |
| this range overlaps on `29_u8`...
|
= note: you likely meant to write mutually exclusive ranges
= note: `#[warn(overlapping_range_endpoints)]` on by default
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:30:13
|
LL | m!(0u8, 20..30, 31..=40);
| ^^^^^^ ------- this could appear to continue range `20_u8..30_u8`, but `30_u8` isn't matched by either of them
| |
| this range doesn't match `30_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `20_u8..=30_u8`
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:32:13
|
LL | m!(0u8, 20..30, 31..=32);
| ^^^^^^ ------- this could appear to continue range `20_u8..30_u8`, but `30_u8` isn't matched by either of them
| |
| this range doesn't match `30_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `20_u8..=30_u8`
error: exclusive range missing `u8::MAX`
--> $DIR/gap_between_ranges.rs:42:9
|
LL | 0..255 => {}
| ^^^^^^
| |
| this range doesn't match `u8::MAX` because `..` is an exclusive range
| help: use an inclusive range instead: `0_u8..=u8::MAX`
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:71:9
|
LL | 0..10 => {}
| ^^^^^
| |
| this range doesn't match `10_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `0_u8..=10_u8`
LL | 11..20 => {}
| ------ this could appear to continue range `0_u8..10_u8`, but `10_u8` isn't matched by either of them
LL | 11..30 => {}
| ------ this could appear to continue range `0_u8..10_u8`, but `10_u8` isn't matched by either of them
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:77:9
|
LL | 0..10 => {}
| ^^^^^
| |
| this range doesn't match `10_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `0_u8..=10_u8`
LL | 11..20 => {}
| ------ this could appear to continue range `0_u8..10_u8`, but `10_u8` isn't matched by either of them
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:78:9
|
LL | 11..20 => {}
| ^^^^^^
| |
| this range doesn't match `20_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `11_u8..=20_u8`
LL | 21..30 => {}
| ------ this could appear to continue range `11_u8..20_u8`, but `20_u8` isn't matched by either of them
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:83:9
|
LL | 00..20 => {}
| ^^^^^^
| |
| this range doesn't match `20_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `0_u8..=20_u8`
LL | 10..20 => {}
LL | 21..30 => {}
| ------ this could appear to continue range `0_u8..20_u8`, but `20_u8` isn't matched by either of them
LL | 21..40 => {}
| ------ this could appear to continue range `0_u8..20_u8`, but `20_u8` isn't matched by either of them
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:84:9
|
LL | 10..20 => {}
| ^^^^^^
| |
| this range doesn't match `20_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `10_u8..=20_u8`
LL | 21..30 => {}
| ------ this could appear to continue range `10_u8..20_u8`, but `20_u8` isn't matched by either of them
LL | 21..40 => {}
| ------ this could appear to continue range `10_u8..20_u8`, but `20_u8` isn't matched by either of them
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:92:10
|
LL | (0..10, true) => {}
| ^^^^^
| |
| this range doesn't match `10_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `0_u8..=10_u8`
LL | (11..20, true) => {}
| ------ this could appear to continue range `0_u8..10_u8`, but `10_u8` isn't matched by either of them
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:97:16
|
LL | (true, 0..10) => {}
| ^^^^^
| |
| this range doesn't match `10_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `0_u8..=10_u8`
LL | (true, 11..20) => {}
| ------ this could appear to continue range `0_u8..10_u8`, but `10_u8` isn't matched by either of them
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:103:10
|
LL | (0..10, true) => {}
| ^^^^^
| |
| this range doesn't match `10_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `0_u8..=10_u8`
LL | (11..20, false) => {}
| ------ this could appear to continue range `0_u8..10_u8`, but `10_u8` isn't matched by either of them
error: multiple ranges are one apart
--> $DIR/gap_between_ranges.rs:113:14
|
LL | Some(0..10) => {}
| ^^^^^
| |
| this range doesn't match `10_u8` because `..` is an exclusive range
| help: use an inclusive range instead: `0_u8..=10_u8`
LL | Some(11..20) => {}
| ------ this could appear to continue range `0_u8..10_u8`, but `10_u8` isn't matched by either of them
error: aborting due to 16 previous errors; 1 warning emitted