Lint overlapping ranges as a separate pass

Author: Nadrieril

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

@@ -53,22 +53,20 @@ use smallvec::{smallvec, SmallVec};
 use rustc_apfloat::ieee::{DoubleS, IeeeFloat, SingleS};
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fx::FxHashSet;
-use rustc_hir::{HirId, RangeEnd};
+use rustc_hir::RangeEnd;
 use rustc_index::Idx;
 use rustc_middle::middle::stability::EvalResult;
 use rustc_middle::mir;
 use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange};
 use rustc_middle::ty::layout::IntegerExt;
 use rustc_middle::ty::{self, Ty, TyCtxt, VariantDef};
-use rustc_session::lint;
 use rustc_span::{Span, DUMMY_SP};
 use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT};
 
 use self::Constructor::*;
 use self::SliceKind::*;
 
 use super::usefulness::{MatchCheckCtxt, PatCtxt};
-use crate::errors::{Overlap, OverlappingRangeEndpoints};
 
 /// Recursively expand this pattern into its subpatterns. Only useful for or-patterns.
 fn expand_or_pat<'p, 'tcx>(pat: &'p Pat<'tcx>) -> Vec<&'p Pat<'tcx>> {
@@ -111,15 +109,15 @@ pub(crate) struct IntRange {
 
 impl IntRange {
     #[inline]
-    fn is_integral(ty: Ty<'_>) -> bool {
+    pub(super) fn is_integral(ty: Ty<'_>) -> bool {
         matches!(ty.kind(), ty::Char | ty::Int(_) | ty::Uint(_) | ty::Bool)
     }
 
-    fn is_singleton(&self) -> bool {
+    pub(super) fn is_singleton(&self) -> bool {
         self.range.start() == self.range.end()
     }
 
-    fn boundaries(&self) -> (u128, u128) {
+    pub(super) fn boundaries(&self) -> (u128, u128) {
         (*self.range.start(), *self.range.end())
     }
 
@@ -177,23 +175,6 @@ impl IntRange {
         }
     }
 
-    fn suspicious_intersection(&self, other: &Self) -> bool {
-        // `false` in the following cases:
-        // 1     ----      // 1  ----------   // 1 ----        // 1       ----
-        // 2  ----------   // 2     ----      // 2       ----  // 2 ----
-        //
-        // The following are currently `false`, but could be `true` in the future (#64007):
-        // 1 ---------       // 1     ---------
-        // 2     ----------  // 2 ----------
-        //
-        // `true` in the following cases:
-        // 1 -------          // 1       -------
-        // 2       --------   // 2 -------
-        let (lo, hi) = self.boundaries();
-        let (other_lo, other_hi) = other.boundaries();
-        (lo == other_hi || hi == other_lo) && !self.is_singleton() && !other.is_singleton()
-    }
-
     /// Partition a range of integers into disjoint subranges. This does constructor splitting for
     /// integer ranges as explained at the top of the file.
     ///
@@ -293,7 +274,7 @@ impl IntRange {
     }
 
     /// Only used for displaying the range.
-    fn to_pat<'tcx>(&self, tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Pat<'tcx> {
+    pub(super) fn to_pat<'tcx>(&self, tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Pat<'tcx> {
         let (lo, hi) = self.boundaries();
 
         let bias = IntRange::signed_bias(tcx, ty);
@@ -315,51 +296,6 @@ impl IntRange {
 
         Pat { ty, span: DUMMY_SP, kind }
     }
-
-    /// Lint on likely incorrect range patterns (#63987)
-    pub(super) fn lint_overlapping_range_endpoints<'a, 'p: 'a, 'tcx: 'a>(
-        &self,
-        pcx: &PatCtxt<'_, 'p, 'tcx>,
-        pats: impl Iterator<Item = &'a DeconstructedPat<'p, 'tcx>>,
-        column_count: usize,
-        lint_root: HirId,
-    ) {
-        if self.is_singleton() {
-            return;
-        }
-
-        if column_count != 1 {
-            // FIXME: for now, only check for overlapping ranges on simple range
-            // patterns. Otherwise with the current logic the following is detected
-            // as overlapping:
-            // ```
-            // match (0u8, true) {
-            //   (0 ..= 125, false) => {}
-            //   (125 ..= 255, true) => {}
-            //   _ => {}
-            // }
-            // ```
-            return;
-        }
-
-        let overlap: Vec<_> = pats
-            .filter_map(|pat| Some((pat.ctor().as_int_range()?, pat.span())))
-            .filter(|(range, _)| self.suspicious_intersection(range))
-            .map(|(range, span)| Overlap {
-                range: self.intersection(&range).unwrap().to_pat(pcx.cx.tcx, pcx.ty),
-                span,
-            })
-            .collect();
-
-        if !overlap.is_empty() {
-            pcx.cx.tcx.emit_spanned_lint(
-                lint::builtin::OVERLAPPING_RANGE_ENDPOINTS,
-                lint_root,
-                pcx.span,
-                OverlappingRangeEndpoints { overlap, range: pcx.span },
-            );
-        }
-    }
 }
 
 /// 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> {
             _ => None,
         }
     }
-    fn as_int_range(&self) -> Option<&IntRange> {
+    pub(super) fn as_int_range(&self) -> Option<&IntRange> {
         match self {
             IntRange(range) => Some(range),
             _ => None,

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

@@ -308,9 +308,9 @@
 use self::ArmType::*;
 use self::Usefulness::*;
 use super::deconstruct_pat::{
-    Constructor, ConstructorSet, DeconstructedPat, SplitConstructorSet, WitnessPat,
+    Constructor, ConstructorSet, DeconstructedPat, IntRange, SplitConstructorSet, WitnessPat,
 };
-use crate::errors::{NonExhaustiveOmittedPattern, Uncovered};
+use crate::errors::{NonExhaustiveOmittedPattern, Overlap, OverlappingRangeEndpoints, Uncovered};
 
 use rustc_data_structures::captures::Captures;
 
@@ -319,6 +319,7 @@ use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_hir::def_id::DefId;
 use rustc_hir::HirId;
 use rustc_middle::ty::{self, Ty, TyCtxt};
+use rustc_session::lint;
 use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS;
 use rustc_span::{Span, DUMMY_SP};
 
@@ -475,11 +476,6 @@ impl<'p, 'tcx> Matrix<'p, 'tcx> {
         Matrix { patterns: vec![] }
     }
 
-    /// Number of columns of this matrix. `None` is the matrix is empty.
-    pub(super) fn column_count(&self) -> Option<usize> {
-        self.patterns.get(0).map(|r| r.len())
-    }
-
     /// Pushes a new row to the matrix. If the row starts with an or-pattern, this recursively
     /// expands it.
     fn push(&mut self, row: PatStack<'p, 'tcx>) {
@@ -835,15 +831,6 @@ fn is_useful<'p, 'tcx>(
 
         let v_ctor = v.head().ctor();
         debug!(?v_ctor);
-        if let Constructor::IntRange(ctor_range) = &v_ctor {
-            // Lint on likely incorrect range patterns (#63987)
-            ctor_range.lint_overlapping_range_endpoints(
-                pcx,
-                matrix.heads(),
-                matrix.column_count().unwrap_or(0),
-                lint_root,
-            )
-        }
         // We split the head constructor of `v`.
         let split_ctors = v_ctor.split(pcx, matrix.heads().map(DeconstructedPat::ctor));
         // For each constructor, we compute whether there's a value that starts with it that would
@@ -903,6 +890,9 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> {
         let column_ctors = self.patterns.iter().map(|p| p.ctor());
         ConstructorSet::for_ty(pcx.cx, pcx.ty).split(pcx, column_ctors)
     }
+    fn iter<'a>(&'a self) -> impl Iterator<Item = &'p DeconstructedPat<'p, 'tcx>> + Captures<'a> {
+        self.patterns.iter().copied()
+    }
 
     /// Does specialization: given a constructor, this takes the patterns from the column that match
     /// the constructor, and outputs their fields.
@@ -993,6 +983,82 @@ fn collect_nonexhaustive_missing_variants<'p, 'tcx>(
     witnesses
 }
 
+/// Traverse the patterns to warn the user about ranges that overlap on their endpoints.
+#[instrument(level = "debug", skip(cx, lint_root))]
+fn lint_overlapping_range_endpoints<'p, 'tcx>(
+    cx: &MatchCheckCtxt<'p, 'tcx>,
+    column: &PatternColumn<'p, 'tcx>,
+    lint_root: HirId,
+) {
+    if column.is_empty() {
+        return;
+    }
+    let ty = column.head_ty();
+    let pcx = &PatCtxt { cx, ty, span: DUMMY_SP, is_top_level: false };
+
+    let set = column.analyze_ctors(pcx);
+
+    if IntRange::is_integral(ty) {
+        let emit_lint = |overlap: &IntRange, this_span: Span, overlapped_spans: &[Span]| {
+            let overlap_as_pat = overlap.to_pat(cx.tcx, ty);
+            let overlaps: Vec<_> = overlapped_spans
+                .iter()
+                .copied()
+                .map(|span| Overlap { range: overlap_as_pat.clone(), span })
+                .collect();
+            cx.tcx.emit_spanned_lint(
+                lint::builtin::OVERLAPPING_RANGE_ENDPOINTS,
+                lint_root,
+                this_span,
+                OverlappingRangeEndpoints { overlap: overlaps, range: this_span },
+            );
+        };
+
+        // If two ranges overlapped, the split set will contain their intersection as a singleton.
+        let split_int_ranges = set.present.iter().filter_map(|c| c.as_int_range());
+        for overlap_range in split_int_ranges.clone() {
+            if overlap_range.is_singleton() {
+                let overlap: u128 = overlap_range.boundaries().0;
+                // Spans of ranges that start or end with the overlap.
+                let mut prefixes: SmallVec<[_; 1]> = Default::default();
+                let mut suffixes: SmallVec<[_; 1]> = Default::default();
+                // Iterate on patterns that contained `overlap`.
+                for pat in column.iter() {
+                    let this_span = pat.span();
+                    let Constructor::IntRange(this_range) = pat.ctor() else { continue };
+                    if this_range.is_singleton() {
+                        // Don't lint when one of the ranges is a singleton.
+                        continue;
+                    }
+                    let (start, end) = this_range.boundaries();
+                    if start == overlap {
+                        // `this_range` looks like `overlap..=end`; it overlaps with any ranges that
+                        // look like `start..=overlap`.
+                        if !prefixes.is_empty() {
+                            emit_lint(overlap_range, this_span, &prefixes);
+                        }
+                        suffixes.push(this_span)
+                    } else if end == overlap {
+                        // `this_range` looks like `start..=overlap`; it overlaps with any ranges
+                        // that look like `overlap..=end`.
+                        if !suffixes.is_empty() {
+                            emit_lint(overlap_range, this_span, &suffixes);
+                        }
+                        prefixes.push(this_span)
+                    }
+                }
+            }
+        }
+    } else {
+        // Recurse into the fields.
+        for ctor in set.present {
+            for col in column.specialize(pcx, &ctor) {
+                lint_overlapping_range_endpoints(cx, &col, lint_root);
+            }
+        }
+    }
+}
+
 /// The arm of a match expression.
 #[derive(Clone, Copy, Debug)]
 pub(crate) struct MatchArm<'p, 'tcx> {
@@ -1063,6 +1129,10 @@ pub(crate) fn compute_match_usefulness<'p, 'tcx>(
         NoWitnesses { .. } => bug!(),
     };
 
+    let pat_column = arms.iter().flat_map(|arm| arm.pat.flatten_or_pat()).collect::<Vec<_>>();
+    let pat_column = PatternColumn::new(pat_column);
+    lint_overlapping_range_endpoints(cx, &pat_column, lint_root);
+
     // Run the non_exhaustive_omitted_patterns lint. Only run on refutable patterns to avoid hitting
     // `if let`s. Only run if the match is exhaustive otherwise the error is redundant.
     if cx.refutable
@@ -1072,10 +1142,7 @@ pub(crate) fn compute_match_usefulness<'p, 'tcx>(
             rustc_session::lint::Level::Allow
         )
     {
-        let pat_column = arms.iter().flat_map(|arm| arm.pat.flatten_or_pat()).collect::<Vec<_>>();
-        let pat_column = PatternColumn::new(pat_column);
         let witnesses = collect_nonexhaustive_missing_variants(cx, &pat_column);
-
         if !witnesses.is_empty() {
             // Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns`
             // is not exhaustive enough.

tests/ui/mir/mir_match_test.rs

@@ -1,4 +1,5 @@
 #![feature(exclusive_range_pattern)]
+#![allow(overlapping_range_endpoints)]
 
 // run-pass
 

tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs

@@ -8,17 +8,17 @@ macro_rules! m {
             $t2 => {}
             _ => {}
         }
-    }
+    };
 }
 
 fn main() {
     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 overlap on their endpoints
-    m!(0u8, 20.. 30, 28..=40);
-    m!(0u8, 20.. 30, 30..=40);
+    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 overlap on their endpoints
     m!(0u8, 20..=30, 29..=30);
@@ -28,27 +28,29 @@ fn main() {
     m!(0u8, 20..=30, 20);
     m!(0u8, 20..=30, 25);
     m!(0u8, 20..=30, 30);
-    m!(0u8, 20.. 30, 29);
+    m!(0u8, 20..30, 29);
     m!(0u8, 20, 20..=30);
     m!(0u8, 25, 20..=30);
     m!(0u8, 30, 20..=30);
 
     match 0u8 {
         0..=10 => {}
         20..=30 => {}
-        10..=20 => {} //~ ERROR multiple patterns overlap on their endpoints
+        10..=20 => {}
+        //~^ ERROR multiple patterns overlap on their endpoints
+        //~| ERROR multiple patterns overlap on their endpoints
         _ => {}
     }
     match (0u8, true) {
         (0..=10, true) => {}
-        (10..20, true) => {} // not detected
-        (10..20, false) => {}
+        (10..20, true) => {} //~ ERROR multiple patterns overlap on their endpoints
+        (10..20, false) => {} //~ ERROR multiple patterns overlap on their endpoints
         _ => {}
     }
     match (true, 0u8) {
         (true, 0..=10) => {}
         (true, 10..20) => {} //~ ERROR multiple patterns overlap on their endpoints
-        (false, 10..20) => {}
+        (false, 10..20) => {} //~ ERROR multiple patterns overlap on their endpoints
         _ => {}
     }
     match Some(0u8) {