Only lint ranges that really overlap

Author: Nadrieril

compiler/rustc_pattern_analysis/src/lib.rs

@@ -122,7 +122,9 @@ pub fn analyze_match<'p, 'tcx>(
     let pat_column = PatternColumn::new(arms);
 
     // Lint ranges that overlap on their endpoints, which is likely a mistake.
-    lint_overlapping_range_endpoints(cx, &pat_column);
+    if !report.overlapping_range_endpoints.is_empty() {
+        lint_overlapping_range_endpoints(cx, &report.overlapping_range_endpoints);
+    }
 
     // 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.

compiler/rustc_pattern_analysis/src/lints.rs

@@ -1,11 +1,7 @@
-use smallvec::SmallVec;
-
-use rustc_data_structures::captures::Captures;
-use rustc_middle::ty::{self, Ty};
+use rustc_middle::ty::Ty;
 use rustc_session::lint;
 use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS;
 
-use crate::constructor::MaybeInfiniteInt;
 use crate::errors::{
     NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Overlap,
     OverlappingRangeEndpoints, Uncovered,
@@ -14,7 +10,6 @@ use crate::rustc::{
     self, Constructor, DeconstructedPat, MatchArm, MatchCtxt, PlaceCtxt, RustcMatchCheckCtxt,
     SplitConstructorSet, WitnessPat,
 };
-use crate::usefulness::OverlappingRanges;
 use crate::TypeCx;
 
 /// A column of patterns in the matrix, where a column is the intuitive notion of "subpatterns that
@@ -64,10 +59,6 @@ impl<'p, 'tcx> PatternColumn<'p, 'tcx> {
         pcx.ctors_for_ty().split(pcx, column_ctors)
     }
 
-    fn iter(&self) -> impl Iterator<Item = &'p DeconstructedPat<'p, 'tcx>> + Captures<'_> {
-        self.patterns.iter().copied()
-    }
-
     /// Does specialization: given a constructor, this takes the patterns from the column that match
     /// the constructor, and outputs their fields.
     /// This returns one column per field of the constructor. They usually all have the same length
@@ -212,81 +203,10 @@ pub(crate) fn lint_nonexhaustive_missing_variants<'a, 'p, 'tcx>(
     }
 }
 
-/// Traverse the patterns to warn the user about ranges that overlap on their endpoints.
-#[instrument(level = "debug", skip(cx))]
-pub(crate) fn collect_overlapping_range_endpoints<'a, 'p, 'tcx>(
-    cx: MatchCtxt<'a, 'p, 'tcx>,
-    column: &PatternColumn<'p, 'tcx>,
-    overlapping_range_endpoints: &mut Vec<rustc::OverlappingRanges<'p, 'tcx>>,
-) {
-    let Some(ty) = column.head_ty(cx) else {
-        return;
-    };
-    let pcx = &PlaceCtxt::new_dummy(cx, ty);
-
-    let set = column.analyze_ctors(pcx);
-
-    if matches!(ty.kind(), ty::Char | ty::Int(_) | ty::Uint(_)) {
-        // 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: MaybeInfiniteInt = overlap_range.lo;
-                // Ranges that look like `lo..=overlap`.
-                let mut prefixes: SmallVec<[_; 1]> = Default::default();
-                // Ranges that look like `overlap..=hi`.
-                let mut suffixes: SmallVec<[_; 1]> = Default::default();
-                // Iterate on patterns that contained `overlap`.
-                for pat in column.iter() {
-                    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;
-                    }
-                    if this_range.lo == overlap {
-                        // `this_range` looks like `overlap..=this_range.hi`; it overlaps with any
-                        // ranges that look like `lo..=overlap`.
-                        if !prefixes.is_empty() {
-                            overlapping_range_endpoints.push(OverlappingRanges {
-                                pat,
-                                overlaps_on: *overlap_range,
-                                overlaps_with: prefixes.as_slice().to_vec(),
-                            });
-                        }
-                        suffixes.push(pat)
-                    } else if this_range.hi == overlap.plus_one() {
-                        // `this_range` looks like `this_range.lo..=overlap`; it overlaps with any
-                        // ranges that look like `overlap..=hi`.
-                        if !suffixes.is_empty() {
-                            overlapping_range_endpoints.push(OverlappingRanges {
-                                pat,
-                                overlaps_on: *overlap_range,
-                                overlaps_with: suffixes.as_slice().to_vec(),
-                            });
-                        }
-                        prefixes.push(pat)
-                    }
-                }
-            }
-        }
-    } else {
-        // Recurse into the fields.
-        for ctor in set.present {
-            for col in column.specialize(pcx, &ctor) {
-                collect_overlapping_range_endpoints(cx, &col, overlapping_range_endpoints);
-            }
-        }
-    }
-}
-
-#[instrument(level = "debug", skip(cx))]
 pub(crate) fn lint_overlapping_range_endpoints<'a, 'p, 'tcx>(
     cx: MatchCtxt<'a, 'p, 'tcx>,
-    column: &PatternColumn<'p, 'tcx>,
+    overlapping_range_endpoints: &[rustc::OverlappingRanges<'p, 'tcx>],
 ) {
-    let mut overlapping_range_endpoints = Vec::new();
-    collect_overlapping_range_endpoints(cx, column, &mut overlapping_range_endpoints);
-
     let rcx = cx.tycx;
     for overlap in overlapping_range_endpoints {
         let overlap_as_pat = rcx.hoist_pat_range(&overlap.overlaps_on, overlap.pat.ty());

compiler/rustc_pattern_analysis/src/usefulness.rs

@@ -1344,6 +1344,7 @@ impl<Cx: TypeCx> WitnessMatrix<Cx> {
 fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
     mcx: MatchCtxt<'a, 'p, Cx>,
     matrix: &mut Matrix<'p, Cx>,
+    overlapping_range_endpoints: &mut Vec<OverlappingRanges<'p, Cx>>,
     is_top_level: bool,
 ) -> WitnessMatrix<Cx> {
     debug_assert!(matrix.rows().all(|r| r.len() == matrix.column_count()));
@@ -1423,30 +1424,102 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
         let ctor_is_relevant = matches!(ctor, Constructor::Missing) || missing_ctors.is_empty();
         let mut spec_matrix = matrix.specialize_constructor(pcx, &ctor, ctor_is_relevant);
         let mut witnesses = ensure_sufficient_stack(|| {
-            compute_exhaustiveness_and_usefulness(mcx, &mut spec_matrix, false)
+            compute_exhaustiveness_and_usefulness(
+                mcx,
+                &mut spec_matrix,
+                overlapping_range_endpoints,
+                false,
+            )
         });
 
         // Transform witnesses for `spec_matrix` into witnesses for `matrix`.
         witnesses.apply_constructor(pcx, &missing_ctors, &ctor, report_individual_missing_ctors);
         // Accumulate the found witnesses.
         ret.extend(witnesses);
 
-        for (child_row_id, child_row) in spec_matrix.rows().enumerate() {
+        for child_row in spec_matrix.rows() {
             let parent_row_id = child_row.parent_row;
             let parent_row = &mut matrix.rows[parent_row_id];
             // A parent row is useful if any of its children is.
             parent_row.useful |= child_row.useful;
             for child_intersects in child_row.intersects.iter() {
                 // Convert the intersecting ids into ids for the parent matrix.
                 let parent_intersects = spec_matrix.rows[child_intersects].parent_row;
-                debug!("child row {child_row_id} intersects with child row {child_intersects}");
-                debug!("parent row {parent_row_id} intersects with parent row {parent_intersects}");
+                // Note: self-intersection can happen with or-patterns.
                 if parent_intersects != parent_row_id {
-                    // self-intersect can happen with or-patterns
                     parent_row.intersects.insert(parent_intersects);
                 }
             }
         }
+
+        // Detect ranges that overlap on their endpoints.
+        if let Constructor::IntRange(overlap_range) = ctor {
+            // If two ranges overlap on their endpoing, that endpoint will show up as a singleton in
+            // the splitted set.
+            if overlap_range.is_singleton()
+                && spec_matrix.rows.len() >= 2
+                && spec_matrix.rows.iter().any(|row| !row.intersects.is_empty())
+            {
+                let overlap = overlap_range.lo;
+                // Ranges that look like `lo..=overlap`.
+                let mut prefixes: SmallVec<[_; 1]> = Default::default();
+                // Ranges that look like `overlap..=hi`.
+                let mut suffixes: SmallVec<[_; 1]> = Default::default();
+                // Iterate on patterns that contained `overlap`. We iterate on `spec_matrix` which
+                // contains only rows that match the current `ctor` as well as accurate intersection
+                // information. It doesn't contain the range; those are in `matrix`.
+                for (child_row_id, child_row) in spec_matrix.rows().enumerate() {
+                    let parent_row = &matrix.rows[child_row.parent_row];
+                    let pat = parent_row.head();
+                    let Constructor::IntRange(this_range) = pat.ctor() else { continue };
+                    // Don't lint when one of the ranges is a singleton.
+                    if this_range.is_singleton() {
+                        continue;
+                    }
+                    if this_range.lo == overlap {
+                        // `this_range` looks like `overlap..=this_range.hi`; it overlaps with any
+                        // ranges that look like `lo..=overlap`.
+                        if !prefixes.is_empty() {
+                            let overlaps_with: Vec<_> = prefixes
+                                .iter()
+                                .filter(|&&(other_child_row_id, _)| {
+                                    child_row.intersects.contains(other_child_row_id)
+                                })
+                                .map(|&(_, pat)| pat)
+                                .collect();
+                            if !overlaps_with.is_empty() {
+                                overlapping_range_endpoints.push(OverlappingRanges {
+                                    pat,
+                                    overlaps_on: overlap_range,
+                                    overlaps_with,
+                                });
+                            }
+                        }
+                        suffixes.push((child_row_id, pat))
+                    } else if this_range.hi == overlap.plus_one() {
+                        // `this_range` looks like `this_range.lo..=overlap`; it overlaps with any
+                        // ranges that look like `overlap..=hi`.
+                        if !suffixes.is_empty() {
+                            let overlaps_with: Vec<_> = suffixes
+                                .iter()
+                                .filter(|&&(other_child_row_id, _)| {
+                                    child_row.intersects.contains(other_child_row_id)
+                                })
+                                .map(|&(_, pat)| pat)
+                                .collect();
+                            if !overlaps_with.is_empty() {
+                                overlapping_range_endpoints.push(OverlappingRanges {
+                                    pat,
+                                    overlaps_on: overlap_range,
+                                    overlaps_with,
+                                });
+                            }
+                        }
+                        prefixes.push((child_row_id, pat))
+                    }
+                }
+            }
+        }
     }
 
     // Record usefulness in the patterns.
@@ -1486,6 +1559,7 @@ pub struct UsefulnessReport<'p, Cx: TypeCx> {
     /// If the match is exhaustive, this is empty. If not, this contains witnesses for the lack of
     /// exhaustiveness.
     pub non_exhaustiveness_witnesses: Vec<WitnessPat<Cx>>,
+    pub overlapping_range_endpoints: Vec<OverlappingRanges<'p, Cx>>,
 }
 
 /// Computes whether a match is exhaustive and which of its arms are useful.
@@ -1496,8 +1570,14 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>(
     scrut_ty: Cx::Ty,
     scrut_validity: ValidityConstraint,
 ) -> UsefulnessReport<'p, Cx> {
+    let mut overlapping_range_endpoints = Vec::new();
     let mut matrix = Matrix::new(cx.wildcard_arena, arms, scrut_ty, scrut_validity);
-    let non_exhaustiveness_witnesses = compute_exhaustiveness_and_usefulness(cx, &mut matrix, true);
+    let non_exhaustiveness_witnesses = compute_exhaustiveness_and_usefulness(
+        cx,
+        &mut matrix,
+        &mut overlapping_range_endpoints,
+        true,
+    );
 
     let non_exhaustiveness_witnesses: Vec<_> = non_exhaustiveness_witnesses.single_column();
     let arm_usefulness: Vec<_> = arms
@@ -1514,5 +1594,5 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>(
             (arm, usefulness)
         })
         .collect();
-    UsefulnessReport { arm_usefulness, non_exhaustiveness_witnesses }
+    UsefulnessReport { arm_usefulness, non_exhaustiveness_witnesses, overlapping_range_endpoints }
 }

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

@@ -44,13 +44,13 @@ fn main() {
     match (0u8, true) {
         (0..=10, true) => {}
         (10..20, true) => {} //~ ERROR multiple patterns overlap on their endpoints
-        (10..20, false) => {} //~ ERROR multiple patterns overlap on their endpoints
+        (10..20, false) => {}
         _ => {}
     }
     match (true, 0u8) {
         (true, 0..=10) => {}
         (true, 10..20) => {} //~ ERROR multiple patterns overlap on their endpoints
-        (false, 10..20) => {} //~ ERROR multiple patterns overlap on their endpoints
+        (false, 10..20) => {}
         _ => {}
     }
     match Some(0u8) {

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

@@ -84,17 +84,6 @@ LL |         (10..20, true) => {}
    |
    = note: you likely meant to write mutually exclusive ranges
 
-error: multiple patterns overlap on their endpoints
-  --> $DIR/overlapping_range_endpoints.rs:47:10
-   |
-LL |         (0..=10, true) => {}
-   |          ------ this range overlaps on `10_u8`...
-LL |         (10..20, true) => {}
-LL |         (10..20, false) => {}
-   |          ^^^^^^ ... with this range
-   |
-   = note: you likely meant to write mutually exclusive ranges
-
 error: multiple patterns overlap on their endpoints
   --> $DIR/overlapping_range_endpoints.rs:52:16
    |
@@ -105,17 +94,6 @@ 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:53:17
-   |
-LL |         (true, 0..=10) => {}
-   |                ------ this range overlaps on `10_u8`...
-LL |         (true, 10..20) => {}
-LL |         (false, 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:58:14
    |
@@ -126,5 +104,5 @@ LL |         Some(10..20) => {}
    |
    = note: you likely meant to write mutually exclusive ranges
 
-error: aborting due to 12 previous errors
+error: aborting due to 10 previous errors