better range analysis: do overlap check after splitting

Nadrieril · Nadrieril · commit 4ef4be691632 · 2023-10-15T19:34:38.000+02:00
diff --git a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
@@ -177,23 +177,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.
     ///
@@ -320,44 +303,63 @@ impl IntRange {
     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>>,
+        pats: impl Iterator<Item = (&'a DeconstructedPat<'p, 'tcx>, bool)>,
         column_count: usize,
         lint_root: HirId,
     ) {
-        if self.is_singleton() {
+        // FIXME: for now, only check for overlapping ranges on non-nested range patterns. Otherwise
+        // with the current logic the following is detected as overlapping:
+        // ```
+        // match (0u8, true) {
+        //   (0 ..= 125, false) => {}
+        //   (125 ..= 255, true) => {}
+        //   _ => {}
+        // }
+        // ```
+        if !self.is_singleton() || column_count != 1 {
             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 },
-            );
+        let overlap: u128 = self.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 rows that contained `overlap`.
+        for (range, this_span, is_under_guard) in pats.filter_map(|(pat, under_guard)| {
+            Some((pat.ctor().as_int_range()?, pat.span(), under_guard))
+        }) {
+            if range.is_singleton() {
+                continue;
+            }
+            let mut overlaps: SmallVec<[_; 1]> = Default::default();
+            if *range.range.start() == overlap {
+                if !prefixes.is_empty() {
+                    overlaps = prefixes.clone();
+                }
+                if !is_under_guard {
+                    suffixes.push(this_span)
+                }
+            } else if *range.range.end() == overlap {
+                if !suffixes.is_empty() {
+                    overlaps = suffixes.clone();
+                }
+                if !is_under_guard {
+                    prefixes.push(this_span)
+                }
+            }
+            if !overlaps.is_empty() {
+                let overlap_as_pat = self.to_pat(pcx.cx.tcx, pcx.ty);
+                let overlaps: Vec<_> = overlaps
+                    .into_iter()
+                    .map(|span| Overlap { range: overlap_as_pat.clone(), span })
+                    .collect();
+                pcx.cx.tcx.emit_spanned_lint(
+                    lint::builtin::OVERLAPPING_RANGE_ENDPOINTS,
+                    lint_root,
+                    this_span,
+                    OverlappingRangeEndpoints { overlap: overlaps, range: this_span },
+                );
+            }
         }
     }
 }
diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
@@ -308,9 +308,8 @@
 use super::deconstruct_pat::{Constructor, ConstructorSet, DeconstructedPat, WitnessPat};
 use crate::errors::{NonExhaustiveOmittedPattern, Uncovered};
 
-use rustc_data_structures::captures::Captures;
-
 use rustc_arena::TypedArena;
+use rustc_data_structures::captures::Captures;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_hir::def_id::DefId;
 use rustc_hir::HirId;
@@ -471,10 +470,6 @@ impl<'p, 'tcx> Matrix<'p, 'tcx> {
         Matrix { rows: vec![], wildcard_row }
     }
 
-    fn len(&self) -> usize {
-        self.rows.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>) {
@@ -813,33 +808,10 @@ fn compute_usefulness<'p, 'tcx>(
     let report_when_all_missing =
         is_top_level && !super::deconstruct_pat::IntRange::is_integral(pcx.ty);
 
-    if super::deconstruct_pat::IntRange::is_integral(ty) {
-        for row_id in 0..matrix.len() {
-            let v = &matrix.rows[row_id];
-            if let Constructor::IntRange(ctor_range) = v.head().ctor() {
-                if ctor_range.is_singleton() {
-                    continue;
-                }
-                // Lint on likely incorrect range patterns (#63987)
-                let pcx = &PatCtxt { span: v.head().span(), ..*pcx };
-                let compare_against = matrix
-                    .rows()
-                    .take(row_id)
-                    .filter(|row| !row.is_under_guard)
-                    .map(|row| row.head());
-                ctor_range.lint_overlapping_range_endpoints(
-                    pcx,
-                    compare_against,
-                    v.len(),
-                    lint_root,
-                )
-            }
-        }
-    }
-
     // For each constructor, we compute whether there's a value that starts with it that would
     // witness the usefulness of `v`.
     let mut ret = WitnessMatrix::new_empty();
+    let orig_column_count = matrix.column_count();
     for ctor in split_ctors {
         debug!("specialize({:?})", ctor);
         let mut spec_matrix = matrix.specialize_constructor(pcx, &ctor);
@@ -848,6 +820,26 @@ fn compute_usefulness<'p, 'tcx>(
         witnesses.apply_constructor(pcx, &missing_ctors, &ctor, report_when_all_missing);
         ret.extend(witnesses);
 
+        // Lint on likely incorrect range patterns (#63987)
+        if spec_matrix.rows().len() >= 2 {
+            if let Constructor::IntRange(overlap_range) = ctor {
+                // If two ranges overlap on their boundaries, that boundary will be found as a singleton
+                // range after splitting.
+                // We limit to a single column for now, see `lint_overlapping_range_endpoints`.
+                if overlap_range.is_singleton() && orig_column_count == 1 {
+                    overlap_range.lint_overlapping_range_endpoints(
+                        pcx,
+                        spec_matrix
+                            .rows()
+                            .map(|child_row| &matrix.rows[child_row.parent_row])
+                            .map(|parent_row| (parent_row.head(), parent_row.is_under_guard)),
+                        orig_column_count,
+                        lint_root,
+                    );
+                }
+            }
+        }
+
         for child_row in spec_matrix.rows() {
             let parent_row = &mut matrix.rows[child_row.parent_row];
             parent_row.is_useful = parent_row.is_useful || child_row.is_useful;
diff --git a/tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs b/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,15 +28,28 @@ 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 {
+        1..=10 => {}
         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 {
+        0..=10 => {}
+        10..=20 if true => {} //~ ERROR multiple patterns overlap on their endpoints
+        _ => {}
+    }
+    match 0u8 {
+        0..=10 if true => {}
+        10..=20 => {}
         _ => {}
     }
     match (0u8, true) {
@@ -51,6 +64,13 @@ fn main() {
         (false, 10..20) => {}
         _ => {}
     }
+    // this probably shouldn't lint
+    match (true, 0u8) {
+        (true, 0..=10) => {}
+        (false, _) => {}
+        (_, 10..20) => {} //~ ERROR multiple patterns overlap on their endpoints
+        _ => {}
+    }
     match Some(0u8) {
         Some(0..=10) => {}
         Some(10..20) => {} //~ ERROR multiple patterns overlap on their endpoints
diff --git a/tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr b/tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr
@@ -24,10 +24,10 @@ LL |     m!(0u8, 30..=40, 20..=30);
    = note: you likely meant to write mutually exclusive ranges
 
 error: multiple patterns overlap on their endpoints
-  --> $DIR/overlapping_range_endpoints.rs:19:22
+  --> $DIR/overlapping_range_endpoints.rs:19:21
    |
-LL |     m!(0u8, 20.. 30, 29..=40);
-   |             -------  ^^^^^^^ ... with this range
+LL |     m!(0u8, 20..30, 29..=40);
+   |             ------  ^^^^^^^ ... with this range
    |             |
    |             this range overlaps on `29_u8`...
    |
@@ -54,10 +54,21 @@ LL |     m!(0u8, 20..=30, 19..=20);
    = note: you likely meant to write mutually exclusive ranges
 
 error: multiple patterns overlap on their endpoints
-  --> $DIR/overlapping_range_endpoints.rs:39:9
+  --> $DIR/overlapping_range_endpoints.rs:40:9
    |
+LL |         1..=10 => {}
+   |         ------ this range overlaps on `10_u8`...
 LL |         0..=10 => {}
    |         ------ this range overlaps on `10_u8`...
+LL |         20..=30 => {}
+LL |         10..=20 => {}
+   |         ^^^^^^^ ... with this range
+   |
+   = note: you likely meant to write mutually exclusive ranges
+
+error: multiple patterns overlap on their endpoints
+  --> $DIR/overlapping_range_endpoints.rs:40:9
+   |
 LL |         20..=30 => {}
    |         ------- this range overlaps on `20_u8`...
 LL |         10..=20 => {}
@@ -66,7 +77,17 @@ LL |         10..=20 => {}
    = note: you likely meant to write mutually exclusive ranges
 
 error: multiple patterns overlap on their endpoints
-  --> $DIR/overlapping_range_endpoints.rs:50:16
+  --> $DIR/overlapping_range_endpoints.rs:47:9
+   |
+LL |         0..=10 => {}
+   |         ------ this range overlaps on `10_u8`...
+LL |         10..=20 if true => {}
+   |         ^^^^^^^ ... with this range
+   |
+   = note: you likely meant to write mutually exclusive ranges
+
+error: multiple patterns overlap on their endpoints
+  --> $DIR/overlapping_range_endpoints.rs:63:16
    |
 LL |         (true, 0..=10) => {}
    |                ------ this range overlaps on `10_u8`...
@@ -76,7 +97,18 @@ LL |         (true, 10..20) => {}
    = note: you likely meant to write mutually exclusive ranges
 
 error: multiple patterns overlap on their endpoints
-  --> $DIR/overlapping_range_endpoints.rs:56:14
+  --> $DIR/overlapping_range_endpoints.rs:71:13
+   |
+LL |         (true, 0..=10) => {}
+   |                ------ this range overlaps on `10_u8`...
+LL |         (false, _) => {}
+LL |         (_, 10..20) => {}
+   |             ^^^^^^ ... with this range
+   |
+   = note: you likely meant to write mutually exclusive ranges
+
+error: multiple patterns overlap on their endpoints
+  --> $DIR/overlapping_range_endpoints.rs:76:14
    |
 LL |         Some(0..=10) => {}
    |              ------ this range overlaps on `10_u8`...
@@ -85,5 +117,5 @@ LL |         Some(10..20) => {}
    |
    = note: you likely meant to write mutually exclusive ranges
 
-error: aborting due to 8 previous errors
+error: aborting due to 11 previous errors
 
