Skip to content

Commit 8ac9a04

Browse files
NadrierilNadrieril
committed
Lint small gaps between ranges
1 parent f783043 commit 8ac9a04

File tree

7 files changed

+492
-12
lines changed

7 files changed

+492
-12
lines changed

‎compiler/rustc_pattern_analysis/messages.ftl

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

‎compiler/rustc_pattern_analysis/src/errors.rs

+51
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,57 @@ impl<'tcx> AddToDiagnostic for Overlap<'tcx> {
7676
}
7777
}
7878

79+
#[derive(LintDiagnostic)]
80+
#[diag(pattern_analysis_excluside_range_missing_max)]
81+
pub struct ExclusiveRangeMissingMax<'tcx> {
82+
#[label]
83+
#[suggestion(code = "{suggestion}", applicability = "maybe-incorrect")]
84+
/// This is an exclusive range that looks like `lo..max` (i.e. doesn't match `max`).
85+
pub first_range: Span,
86+
/// Suggest `lo..=max` instead.
87+
pub suggestion: String,
88+
pub max: Pat<'tcx>,
89+
}
90+
91+
#[derive(LintDiagnostic)]
92+
#[diag(pattern_analysis_excluside_range_missing_gap)]
93+
pub struct ExclusiveRangeMissingGap<'tcx> {
94+
#[label]
95+
#[suggestion(code = "{suggestion}", applicability = "maybe-incorrect")]
96+
/// This is an exclusive range that looks like `lo..gap` (i.e. doesn't match `gap`).
97+
pub first_range: Span,
98+
pub gap: Pat<'tcx>,
99+
/// Suggest `lo..=gap` instead.
100+
pub suggestion: String,
101+
#[subdiagnostic]
102+
/// All these ranges skipped over `gap` which we think is probably a mistake.
103+
pub gap_with: Vec<GappedRange<'tcx>>,
104+
}
105+
106+
pub struct GappedRange<'tcx> {
107+
pub span: Span,
108+
pub gap: Pat<'tcx>,
109+
pub first_range: Pat<'tcx>,
110+
}
111+
112+
impl<'tcx> AddToDiagnostic for GappedRange<'tcx> {
113+
fn add_to_diagnostic_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
114+
self,
115+
diag: &mut Diag<'_, G>,
116+
_: F,
117+
) {
118+
let GappedRange { span, gap, first_range } = self;
119+
120+
// FIXME(mejrs) unfortunately `#[derive(LintDiagnostic)]`
121+
// does not support `#[subdiagnostic(eager)]`...
122+
let message = format!(
123+
"this could appear to continue range `{first_range}`, but `{gap}` isn't matched by \
124+
either of them"
125+
);
126+
diag.span_label(span, message);
127+
}
128+
}
129+
79130
#[derive(LintDiagnostic)]
80131
#[diag(pattern_analysis_non_exhaustive_omitted_pattern)]
81132
#[help]

‎compiler/rustc_pattern_analysis/src/lib.rs

+16-7
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,8 @@ use rustc_middle::ty::Ty;
7070
use rustc_span::ErrorGuaranteed;
7171

7272
use crate::constructor::{Constructor, ConstructorSet, IntRange};
73-
#[cfg(feature = "rustc")]
74-
use crate::lints::lint_nonexhaustive_missing_variants;
7573
use crate::pat::DeconstructedPat;
7674
use crate::pat_column::PatternColumn;
77-
#[cfg(feature = "rustc")]
78-
use crate::rustc::RustcMatchCheckCtxt;
79-
#[cfg(feature = "rustc")]
80-
use crate::usefulness::{compute_match_usefulness, ValidityConstraint};
8175

8276
pub trait Captures<'a> {}
8377
impl<'a, T: ?Sized> Captures<'a> for T {}
@@ -145,6 +139,18 @@ pub trait TypeCx: Sized + fmt::Debug {
145139

146140
/// The maximum pattern complexity limit was reached.
147141
fn complexity_exceeded(&self) -> Result<(), Self::Error>;
142+
143+
/// Lint that there is a gap `gap` between `pat` and all of `gapped_with` such that the gap is
144+
/// not matched by another range. If `gapped_with` is empty, then `gap` is `T::MAX`. We only
145+
/// detect singleton gaps.
146+
/// The default implementation does nothing.
147+
fn lint_non_contiguous_range_endpoints(
148+
&self,
149+
_pat: &DeconstructedPat<Self>,
150+
_gap: IntRange,
151+
_gapped_with: &[&DeconstructedPat<Self>],
152+
) {
153+
}
148154
}
149155

150156
/// The arm of a match expression.
@@ -167,11 +173,14 @@ impl<'p, Cx: TypeCx> Copy for MatchArm<'p, Cx> {}
167173
/// useful, and runs some lints.
168174
#[cfg(feature = "rustc")]
169175
pub fn analyze_match<'p, 'tcx>(
170-
tycx: &RustcMatchCheckCtxt<'p, 'tcx>,
176+
tycx: &rustc::RustcMatchCheckCtxt<'p, 'tcx>,
171177
arms: &[rustc::MatchArm<'p, 'tcx>],
172178
scrut_ty: Ty<'tcx>,
173179
pattern_complexity_limit: Option<usize>,
174180
) -> Result<rustc::UsefulnessReport<'p, 'tcx>, ErrorGuaranteed> {
181+
use lints::lint_nonexhaustive_missing_variants;
182+
use usefulness::{compute_match_usefulness, ValidityConstraint};
183+
175184
let scrut_ty = tycx.reveal_opaque_ty(scrut_ty);
176185
let scrut_validity = ValidityConstraint::from_bool(tycx.known_valid_scrutinee);
177186
let report =

‎compiler/rustc_pattern_analysis/src/rustc.rs

+65-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_index::{Idx, IndexVec};
88
use rustc_middle::middle::stability::EvalResult;
99
use rustc_middle::mir::interpret::Scalar;
1010
use rustc_middle::mir::{self, Const};
11-
use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange, PatRangeBoundary};
11+
use rustc_middle::thir::{self, FieldPat, Pat, PatKind, PatRange, PatRangeBoundary};
1212
use rustc_middle::ty::layout::IntegerExt;
1313
use rustc_middle::ty::{self, FieldDef, OpaqueTypeKey, Ty, TyCtxt, TypeVisitableExt, VariantDef};
1414
use rustc_session::lint;
@@ -900,6 +900,70 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> {
900900
let span = self.whole_match_span.unwrap_or(self.scrut_span);
901901
Err(self.tcx.dcx().span_err(span, "reached pattern complexity limit"))
902902
}
903+
904+
fn lint_non_contiguous_range_endpoints(
905+
&self,
906+
pat: &crate::pat::DeconstructedPat<Self>,
907+
gap: IntRange,
908+
gapped_with: &[&crate::pat::DeconstructedPat<Self>],
909+
) {
910+
let Some(&thir_pat) = pat.data() else { return };
911+
let thir::PatKind::Range(range) = &thir_pat.kind else { return };
912+
// Only lint when the left range is an exclusive range.
913+
if range.end != rustc_hir::RangeEnd::Excluded {
914+
return;
915+
}
916+
// `pat` is an exclusive range like `lo..gap`. `gapped_with` contains ranges that start with
917+
// `gap+1`.
918+
let suggested_range: thir::Pat<'_> = {
919+
// Suggest `lo..=gap` instead.
920+
let mut suggested_range = thir_pat.clone();
921+
let thir::PatKind::Range(range) = &mut suggested_range.kind else { unreachable!() };
922+
range.end = rustc_hir::RangeEnd::Included;
923+
suggested_range
924+
};
925+
let gap_as_pat = self.hoist_pat_range(&gap, *pat.ty());
926+
if gapped_with.is_empty() {
927+
// If `gapped_with` is empty, `gap == T::MAX`.
928+
self.tcx.emit_node_span_lint(
929+
lint::builtin::NON_CONTIGUOUS_RANGE_ENDPOINTS,
930+
self.match_lint_level,
931+
thir_pat.span,
932+
errors::ExclusiveRangeMissingMax {
933+
// Point at this range.
934+
first_range: thir_pat.span,
935+
// That's the gap that isn't covered.
936+
max: gap_as_pat.clone(),
937+
// Suggest `lo..=max` instead.
938+
suggestion: suggested_range.to_string(),
939+
},
940+
);
941+
} else {
942+
self.tcx.emit_node_span_lint(
943+
lint::builtin::NON_CONTIGUOUS_RANGE_ENDPOINTS,
944+
self.match_lint_level,
945+
thir_pat.span,
946+
errors::ExclusiveRangeMissingGap {
947+
// Point at this range.
948+
first_range: thir_pat.span,
949+
// That's the gap that isn't covered.
950+
gap: gap_as_pat.clone(),
951+
// Suggest `lo..=gap` instead.
952+
suggestion: suggested_range.to_string(),
953+
// All these ranges skipped over `gap` which we think is probably a
954+
// mistake.
955+
gap_with: gapped_with
956+
.iter()
957+
.map(|pat| errors::GappedRange {
958+
span: pat.data().unwrap().span,
959+
gap: gap_as_pat.clone(),
960+
first_range: thir_pat.clone(),
961+
})
962+
.collect(),
963+
},
964+
);
965+
}
966+
}
903967
}
904968

905969
/// Recursively expand this pattern into its subpatterns. Only useful for or-patterns.

‎compiler/rustc_pattern_analysis/src/usefulness.rs

+42-4
Original file line numberDiff line numberDiff line change
@@ -1489,7 +1489,7 @@ impl<Cx: TypeCx> WitnessMatrix<Cx> {
14891489
/// We can however get false negatives because exhaustiveness does not explore all cases. See the
14901490
/// section on relevancy at the top of the file.
14911491
fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
1492-
mcx: &mut UsefulnessCtxt<'_, Cx>,
1492+
cx: &Cx,
14931493
overlap_range: IntRange,
14941494
matrix: &Matrix<'p, Cx>,
14951495
specialized_matrix: &Matrix<'p, Cx>,
@@ -1522,7 +1522,7 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
15221522
.map(|&(_, pat)| pat)
15231523
.collect();
15241524
if !overlaps_with.is_empty() {
1525-
mcx.tycx.lint_overlapping_range_endpoints(pat, overlap_range, &overlaps_with);
1525+
cx.lint_overlapping_range_endpoints(pat, overlap_range, &overlaps_with);
15261526
}
15271527
}
15281528
suffixes.push((child_row_id, pat))
@@ -1538,14 +1538,41 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
15381538
.map(|&(_, pat)| pat)
15391539
.collect();
15401540
if !overlaps_with.is_empty() {
1541-
mcx.tycx.lint_overlapping_range_endpoints(pat, overlap_range, &overlaps_with);
1541+
cx.lint_overlapping_range_endpoints(pat, overlap_range, &overlaps_with);
15421542
}
15431543
}
15441544
prefixes.push((child_row_id, pat))
15451545
}
15461546
}
15471547
}
15481548

1549+
/// Collect ranges that have a singleton gap between them.
1550+
fn collect_non_contiguous_range_endpoints<'p, Cx: TypeCx>(
1551+
cx: &Cx,
1552+
gap_range: &IntRange,
1553+
matrix: &Matrix<'p, Cx>,
1554+
) {
1555+
let gap = gap_range.lo;
1556+
// Ranges that look like `lo..gap`.
1557+
let mut onebefore: SmallVec<[_; 1]> = Default::default();
1558+
// Ranges that start on `gap+1` or singletons `gap+1`.
1559+
let mut oneafter: SmallVec<[_; 1]> = Default::default();
1560+
// Look through the column for ranges near the gap.
1561+
for pat in matrix.heads() {
1562+
let PatOrWild::Pat(pat) = pat else { continue };
1563+
let Constructor::IntRange(this_range) = pat.ctor() else { continue };
1564+
if gap == this_range.hi {
1565+
onebefore.push(pat)
1566+
} else if gap.plus_one() == Some(this_range.lo) {
1567+
oneafter.push(pat)
1568+
}
1569+
}
1570+
1571+
for pat_before in onebefore {
1572+
cx.lint_non_contiguous_range_endpoints(pat_before, *gap_range, oneafter.as_slice());
1573+
}
1574+
}
1575+
15491576
/// The core of the algorithm.
15501577
///
15511578
/// 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>(
16261653
&& spec_matrix.rows.len() >= 2
16271654
&& spec_matrix.rows.iter().any(|row| !row.intersects.is_empty())
16281655
{
1629-
collect_overlapping_range_endpoints(mcx, overlap_range, matrix, &spec_matrix);
1656+
collect_overlapping_range_endpoints(mcx.tycx, overlap_range, matrix, &spec_matrix);
16301657
}
16311658
}
16321659

16331660
matrix.unspecialize(spec_matrix);
16341661
}
16351662

1663+
// Detect singleton gaps between ranges.
1664+
if missing_ctors.iter().any(|c| matches!(c, Constructor::IntRange(..))) {
1665+
for missing in &missing_ctors {
1666+
if let Constructor::IntRange(gap) = missing {
1667+
if gap.is_singleton() {
1668+
collect_non_contiguous_range_endpoints(mcx.tycx, gap, matrix);
1669+
}
1670+
}
1671+
}
1672+
}
1673+
16361674
// Record usefulness in the patterns.
16371675
for row in matrix.rows() {
16381676
if row.useful {

0 commit comments

Comments
 (0)