Skip to content

Commit 3bcf078

Browse files
committed
Auto merge of #116751 - Nadrieril:lint-overlap-per-column, r=<try>
Lint overlapping ranges as a separate pass This reworks the [`overlapping_range_endpoints`](https://doc.rust-lang.org/beta/nightly-rustc/rustc_lint_defs/builtin/static.OVERLAPPING_RANGE_ENDPOINTS.html) lint. My motivations are: - It was annoying to have this lint entangled with the exhaustiveness algorithm, especially wrt librarification; - This makes the lint behave consistently. Here's the consistency story. Take the following matches: ```rust match (0u8, true) { (0..=10, true) => {} (10..20, true) => {} (10..20, false) => {} _ => {} } match (true, 0u8) { (true, 0..=10) => {} (true, 10..20) => {} (false, 10..20) => {} _ => {} } ``` There are two semantically consistent options: option 1 we lint all overlaps between the ranges, option 2 we only lint the overlaps that could actually occur (i.e. the ones with `true`). Option 1 is what this PR does. Option 2 is possible but would require the exhaustiveness algorithm to track more things for the sake of the lint. The status quo is that we're inconsistent between the two. Option 1 generates more false postives, but I prefer it from a maintainer's perspective. I do think the difference is minimal; cases where the difference is observable seem rare. This PR adds a separate pass, so this will have a perf impact. Let's see how bad, it looked ok locally.
2 parents ad7b8a0 + 775726f commit 3bcf078

File tree

6 files changed

+257
-146
lines changed

6 files changed

+257
-146
lines changed

compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs

+6-70
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,20 @@ use smallvec::{smallvec, SmallVec};
5353
use rustc_apfloat::ieee::{DoubleS, IeeeFloat, SingleS};
5454
use rustc_data_structures::captures::Captures;
5555
use rustc_data_structures::fx::FxHashSet;
56-
use rustc_hir::{HirId, RangeEnd};
56+
use rustc_hir::RangeEnd;
5757
use rustc_index::Idx;
5858
use rustc_middle::middle::stability::EvalResult;
5959
use rustc_middle::mir;
6060
use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange};
6161
use rustc_middle::ty::layout::IntegerExt;
6262
use rustc_middle::ty::{self, Ty, TyCtxt, VariantDef};
63-
use rustc_session::lint;
6463
use rustc_span::{Span, DUMMY_SP};
6564
use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT};
6665

6766
use self::Constructor::*;
6867
use self::SliceKind::*;
6968

7069
use super::usefulness::{MatchCheckCtxt, PatCtxt};
71-
use crate::errors::{Overlap, OverlappingRangeEndpoints};
7270

7371
/// Recursively expand this pattern into its subpatterns. Only useful for or-patterns.
7472
fn expand_or_pat<'p, 'tcx>(pat: &'p Pat<'tcx>) -> Vec<&'p Pat<'tcx>> {
@@ -111,15 +109,15 @@ pub(crate) struct IntRange {
111109

112110
impl IntRange {
113111
#[inline]
114-
fn is_integral(ty: Ty<'_>) -> bool {
112+
pub(super) fn is_integral(ty: Ty<'_>) -> bool {
115113
matches!(ty.kind(), ty::Char | ty::Int(_) | ty::Uint(_) | ty::Bool)
116114
}
117115

118-
fn is_singleton(&self) -> bool {
116+
pub(super) fn is_singleton(&self) -> bool {
119117
self.range.start() == self.range.end()
120118
}
121119

122-
fn boundaries(&self) -> (u128, u128) {
120+
pub(super) fn boundaries(&self) -> (u128, u128) {
123121
(*self.range.start(), *self.range.end())
124122
}
125123

@@ -177,23 +175,6 @@ impl IntRange {
177175
}
178176
}
179177

180-
fn suspicious_intersection(&self, other: &Self) -> bool {
181-
// `false` in the following cases:
182-
// 1 ---- // 1 ---------- // 1 ---- // 1 ----
183-
// 2 ---------- // 2 ---- // 2 ---- // 2 ----
184-
//
185-
// The following are currently `false`, but could be `true` in the future (#64007):
186-
// 1 --------- // 1 ---------
187-
// 2 ---------- // 2 ----------
188-
//
189-
// `true` in the following cases:
190-
// 1 ------- // 1 -------
191-
// 2 -------- // 2 -------
192-
let (lo, hi) = self.boundaries();
193-
let (other_lo, other_hi) = other.boundaries();
194-
(lo == other_hi || hi == other_lo) && !self.is_singleton() && !other.is_singleton()
195-
}
196-
197178
/// Partition a range of integers into disjoint subranges. This does constructor splitting for
198179
/// integer ranges as explained at the top of the file.
199180
///
@@ -293,7 +274,7 @@ impl IntRange {
293274
}
294275

295276
/// Only used for displaying the range.
296-
fn to_pat<'tcx>(&self, tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Pat<'tcx> {
277+
pub(super) fn to_pat<'tcx>(&self, tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Pat<'tcx> {
297278
let (lo, hi) = self.boundaries();
298279

299280
let bias = IntRange::signed_bias(tcx, ty);
@@ -315,51 +296,6 @@ impl IntRange {
315296

316297
Pat { ty, span: DUMMY_SP, kind }
317298
}
318-
319-
/// Lint on likely incorrect range patterns (#63987)
320-
pub(super) fn lint_overlapping_range_endpoints<'a, 'p: 'a, 'tcx: 'a>(
321-
&self,
322-
pcx: &PatCtxt<'_, 'p, 'tcx>,
323-
pats: impl Iterator<Item = &'a DeconstructedPat<'p, 'tcx>>,
324-
column_count: usize,
325-
lint_root: HirId,
326-
) {
327-
if self.is_singleton() {
328-
return;
329-
}
330-
331-
if column_count != 1 {
332-
// FIXME: for now, only check for overlapping ranges on simple range
333-
// patterns. Otherwise with the current logic the following is detected
334-
// as overlapping:
335-
// ```
336-
// match (0u8, true) {
337-
// (0 ..= 125, false) => {}
338-
// (125 ..= 255, true) => {}
339-
// _ => {}
340-
// }
341-
// ```
342-
return;
343-
}
344-
345-
let overlap: Vec<_> = pats
346-
.filter_map(|pat| Some((pat.ctor().as_int_range()?, pat.span())))
347-
.filter(|(range, _)| self.suspicious_intersection(range))
348-
.map(|(range, span)| Overlap {
349-
range: self.intersection(&range).unwrap().to_pat(pcx.cx.tcx, pcx.ty),
350-
span,
351-
})
352-
.collect();
353-
354-
if !overlap.is_empty() {
355-
pcx.cx.tcx.emit_spanned_lint(
356-
lint::builtin::OVERLAPPING_RANGE_ENDPOINTS,
357-
lint_root,
358-
pcx.span,
359-
OverlappingRangeEndpoints { overlap, range: pcx.span },
360-
);
361-
}
362-
}
363299
}
364300

365301
/// Note: this is often not what we want: e.g. `false` is converted into the range `0..=0` and
@@ -644,7 +580,7 @@ impl<'tcx> Constructor<'tcx> {
644580
_ => None,
645581
}
646582
}
647-
fn as_int_range(&self) -> Option<&IntRange> {
583+
pub(super) fn as_int_range(&self) -> Option<&IntRange> {
648584
match self {
649585
IntRange(range) => Some(range),
650586
_ => None,

0 commit comments

Comments
 (0)