From 89d01babe6d0fd7f66715a4570ed57cada76473d Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 28 Dec 2023 21:59:16 +0100 Subject: [PATCH 1/3] Track row intersections --- .../rustc_pattern_analysis/src/usefulness.rs | 54 ++++++++++++------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index b4935d280e6..06d0a126d47 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -712,6 +712,7 @@ //! I (Nadrieril) prefer to put new tests in `ui/pattern/usefulness` unless there's a specific //! reason not to, for example if they crucially depend on a particular feature like `or_patterns`. +use rustc_index::bit_set::BitSet; use smallvec::{smallvec, SmallVec}; use std::fmt; @@ -911,6 +912,11 @@ struct MatrixRow<'p, Cx: TypeCx> { /// [`compute_exhaustiveness_and_usefulness`] if the arm is found to be useful. /// This is reset to `false` when specializing. useful: bool, + /// Tracks which rows above this one have an intersection with this one, i.e. such that there is + /// a value that matches both rows. + /// Note: Because of relevancy we may miss some intersections. The intersections we do find are + /// correct. + intersects: BitSet, } impl<'p, Cx: TypeCx> MatrixRow<'p, Cx> { @@ -938,6 +944,7 @@ fn expand_or_pat(&self) -> impl Iterator> + Captures<'_ parent_row: self.parent_row, is_under_guard: self.is_under_guard, useful: false, + intersects: BitSet::new_empty(0), // Initialized in `Matrix::expand_and_push`. }) } @@ -955,6 +962,7 @@ fn pop_head_constructor( parent_row, is_under_guard: self.is_under_guard, useful: false, + intersects: BitSet::new_empty(0), // Initialized in `Matrix::expand_and_push`. } } } @@ -993,13 +1001,15 @@ struct Matrix<'p, Cx: TypeCx> { impl<'p, Cx: TypeCx> Matrix<'p, Cx> { /// Pushes a new row to the matrix. If the row starts with an or-pattern, this recursively /// expands it. Internal method, prefer [`Matrix::new`]. - fn expand_and_push(&mut self, row: MatrixRow<'p, Cx>) { + fn expand_and_push(&mut self, mut row: MatrixRow<'p, Cx>) { if !row.is_empty() && row.head().is_or_pat() { // Expand nested or-patterns. - for new_row in row.expand_or_pat() { + for mut new_row in row.expand_or_pat() { + new_row.intersects = BitSet::new_empty(self.rows.len()); self.rows.push(new_row); } } else { + row.intersects = BitSet::new_empty(self.rows.len()); self.rows.push(row); } } @@ -1019,9 +1029,10 @@ fn new( for (row_id, arm) in arms.iter().enumerate() { let v = MatrixRow { pats: PatStack::from_pattern(arm.pat), - parent_row: row_id, // dummy, we won't read it + parent_row: row_id, // dummy, we don't read it is_under_guard: arm.has_guard, useful: false, + intersects: BitSet::new_empty(0), // Initialized in `Matrix::expand_and_push`. }; matrix.expand_and_push(v); } @@ -1349,21 +1360,19 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( let Some(ty) = matrix.head_ty() else { // The base case: there are no columns in the matrix. We are morally pattern-matching on (). // A row is useful iff it has no (unguarded) rows above it. - for row in matrix.rows_mut() { - // All rows are useful until they're not. - row.useful = true; - // When there's an unguarded row, the match is exhaustive and any subsequent row is not - // useful. - if !row.is_under_guard { - return Ok(WitnessMatrix::empty()); - } + let mut useful = true; // Whether the next row is useful. + for (i, row) in matrix.rows_mut().enumerate() { + row.useful = useful; + row.intersects.insert_range(0..i); + // The next rows stays useful if this one is under a guard. + useful &= row.is_under_guard; } - // No (unguarded) rows, so the match is not exhaustive. We return a new witness unless - // irrelevant. - return if matrix.wildcard_row_is_relevant { + return if useful && matrix.wildcard_row_is_relevant { + // The wildcard row is useful; the match is non-exhaustive. Ok(WitnessMatrix::unit_witness()) } else { - // We choose to not report anything here; see at the top for details. + // Either the match is exhaustive, or we choose not to report anything because of + // relevancy. See at the top for details. Ok(WitnessMatrix::empty()) }; }; @@ -1424,10 +1433,19 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( // Accumulate the found witnesses. ret.extend(witnesses); - // A parent row is useful if any of its children is. for child_row in spec_matrix.rows() { - let parent_row = &mut matrix.rows[child_row.parent_row]; - parent_row.useful = parent_row.useful || child_row.useful; + 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_intersection in child_row.intersects.iter() { + // Convert the intersecting ids into ids for the parent matrix. + let parent_intersection = spec_matrix.rows[child_intersection].parent_row; + // Note: self-intersection can happen with or-patterns. + if parent_intersection != parent_row_id { + parent_row.intersects.insert(parent_intersection); + } + } } } From 6f6ba2571d18d24dd041bc46e1ad57496714f117 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 28 Dec 2023 22:22:20 +0100 Subject: [PATCH 2/3] Factor out collection of overlapping ranges --- compiler/rustc_pattern_analysis/src/lib.rs | 2 +- compiler/rustc_pattern_analysis/src/lints.rs | 78 ++++++++++++------- compiler/rustc_pattern_analysis/src/rustc.rs | 2 + .../rustc_pattern_analysis/src/usefulness.rs | 11 ++- 4 files changed, 62 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index b52643adcc9..5d6a0f855f5 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -124,7 +124,7 @@ pub fn analyze_match<'p, 'tcx>( let pat_column = PatternColumn::new(arms); - // Lint on ranges that overlap on their endpoints, which is likely a mistake. + // Lint ranges that overlap on their endpoints, which is likely a mistake. lint_overlapping_range_endpoints(cx, &pat_column)?; // Run the non_exhaustive_omitted_patterns lint. Only run on refutable patterns to avoid hitting diff --git a/compiler/rustc_pattern_analysis/src/lints.rs b/compiler/rustc_pattern_analysis/src/lints.rs index 52c9af85006..941bdad1fd9 100644 --- a/compiler/rustc_pattern_analysis/src/lints.rs +++ b/compiler/rustc_pattern_analysis/src/lints.rs @@ -4,18 +4,18 @@ use rustc_middle::ty; use rustc_session::lint; use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS; -use rustc_span::{ErrorGuaranteed, Span}; +use rustc_span::ErrorGuaranteed; -use crate::constructor::{IntRange, MaybeInfiniteInt}; +use crate::constructor::MaybeInfiniteInt; use crate::errors::{ - NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Overlap, - OverlappingRangeEndpoints, Uncovered, + self, NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered, }; use crate::pat::PatOrWild; use crate::rustc::{ - Constructor, DeconstructedPat, MatchArm, MatchCtxt, PlaceCtxt, RevealedTy, RustcMatchCheckCtxt, - SplitConstructorSet, WitnessPat, + self, Constructor, DeconstructedPat, MatchArm, MatchCtxt, PlaceCtxt, RevealedTy, + RustcMatchCheckCtxt, SplitConstructorSet, WitnessPat, }; +use crate::usefulness::OverlappingRanges; /// A column of patterns in the matrix, where a column is the intuitive notion of "subpatterns that /// inspect the same subvalue/place". @@ -209,34 +209,19 @@ 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 lint_overlapping_range_endpoints<'a, 'p, 'tcx>( +pub(crate) fn collect_overlapping_range_endpoints<'a, 'p, 'tcx>( cx: MatchCtxt<'a, 'p, 'tcx>, column: &PatternColumn<'p, 'tcx>, + overlapping_range_endpoints: &mut Vec>, ) -> Result<(), ErrorGuaranteed> { let Some(ty) = column.head_ty() else { return Ok(()); }; let pcx = &PlaceCtxt::new_dummy(cx, ty); - let rcx: &RustcMatchCheckCtxt<'_, '_> = cx.tycx; let set = column.analyze_ctors(pcx)?; if matches!(ty.kind(), ty::Char | ty::Int(_) | ty::Uint(_)) { - let emit_lint = |overlap: &IntRange, this_span: Span, overlapped_spans: &[Span]| { - let overlap_as_pat = rcx.hoist_pat_range(overlap, ty); - let overlaps: Vec<_> = overlapped_spans - .iter() - .copied() - .map(|span| Overlap { range: overlap_as_pat.clone(), span }) - .collect(); - rcx.tcx.emit_spanned_lint( - lint::builtin::OVERLAPPING_RANGE_ENDPOINTS, - rcx.match_lint_level, - 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() { @@ -249,7 +234,6 @@ pub(crate) fn lint_overlapping_range_endpoints<'a, 'p, 'tcx>( // Iterate on patterns that contained `overlap`. for pat in column.iter() { let Constructor::IntRange(this_range) = pat.ctor() else { continue }; - let this_span = pat.data().unwrap().span; if this_range.is_singleton() { // Don't lint when one of the ranges is a singleton. continue; @@ -258,16 +242,24 @@ pub(crate) fn lint_overlapping_range_endpoints<'a, 'p, 'tcx>( // `this_range` looks like `overlap..=this_range.hi`; it overlaps with any // ranges that look like `lo..=overlap`. if !prefixes.is_empty() { - emit_lint(overlap_range, this_span, &prefixes); + overlapping_range_endpoints.push(OverlappingRanges { + pat, + overlaps_on: *overlap_range, + overlaps_with: prefixes.as_slice().to_vec(), + }); } - suffixes.push(this_span) + 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() { - emit_lint(overlap_range, this_span, &suffixes); + overlapping_range_endpoints.push(OverlappingRanges { + pat, + overlaps_on: *overlap_range, + overlaps_with: suffixes.as_slice().to_vec(), + }); } - prefixes.push(this_span) + prefixes.push(pat) } } } @@ -276,9 +268,37 @@ pub(crate) fn lint_overlapping_range_endpoints<'a, 'p, 'tcx>( // Recurse into the fields. for ctor in set.present { for col in column.specialize(pcx, &ctor) { - lint_overlapping_range_endpoints(cx, &col)?; + collect_overlapping_range_endpoints(cx, &col, overlapping_range_endpoints)?; } } } Ok(()) } + +#[instrument(level = "debug", skip(cx))] +pub(crate) fn lint_overlapping_range_endpoints<'a, 'p, 'tcx>( + cx: MatchCtxt<'a, 'p, 'tcx>, + column: &PatternColumn<'p, 'tcx>, +) -> Result<(), ErrorGuaranteed> { + 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()); + let overlaps: Vec<_> = overlap + .overlaps_with + .iter() + .map(|pat| pat.data().unwrap().span) + .map(|span| errors::Overlap { range: overlap_as_pat.clone(), span }) + .collect(); + let pat_span = overlap.pat.data().unwrap().span; + rcx.tcx.emit_spanned_lint( + lint::builtin::OVERLAPPING_RANGE_ENDPOINTS, + rcx.match_lint_level, + pat_span, + errors::OverlappingRangeEndpoints { overlap: overlaps, range: pat_span }, + ); + } + Ok(()) +} diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index a8d1bece613..a17cd2c81b9 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -34,6 +34,8 @@ crate::pat::DeconstructedPat<'p, RustcMatchCheckCtxt<'p, 'tcx>>; pub type MatchArm<'p, 'tcx> = crate::MatchArm<'p, RustcMatchCheckCtxt<'p, 'tcx>>; pub type MatchCtxt<'a, 'p, 'tcx> = crate::MatchCtxt<'a, 'p, RustcMatchCheckCtxt<'p, 'tcx>>; +pub type OverlappingRanges<'p, 'tcx> = + crate::usefulness::OverlappingRanges<'p, RustcMatchCheckCtxt<'p, 'tcx>>; pub(crate) type PlaceCtxt<'a, 'p, 'tcx> = crate::usefulness::PlaceCtxt<'a, 'p, RustcMatchCheckCtxt<'p, 'tcx>>; pub(crate) type SplitConstructorSet<'p, 'tcx> = diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 06d0a126d47..426c0e8aa86 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -716,7 +716,7 @@ use smallvec::{smallvec, SmallVec}; use std::fmt; -use crate::constructor::{Constructor, ConstructorSet}; +use crate::constructor::{Constructor, ConstructorSet, IntRange}; use crate::pat::{DeconstructedPat, PatOrWild, WitnessPat}; use crate::{Captures, MatchArm, MatchCtxt, TypeCx}; @@ -1471,6 +1471,15 @@ pub enum Usefulness<'p, Cx: TypeCx> { Redundant, } +/// Indicates that the range `pat` overlapped with all the ranges in `overlaps_with`, where the +/// range they overlapped over is `overlaps_on`. We only detect singleton overlaps. +#[derive(Clone, Debug)] +pub struct OverlappingRanges<'p, Cx: TypeCx> { + pub pat: &'p DeconstructedPat<'p, Cx>, + pub overlaps_on: IntRange, + pub overlaps_with: Vec<&'p DeconstructedPat<'p, Cx>>, +} + /// The output of checking a match for exhaustiveness and arm usefulness. pub struct UsefulnessReport<'p, Cx: TypeCx> { /// For each arm of the input, whether that arm is useful after the arms above it. From a24f4db41ba629a2477d6d332585faf5610c9210 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Fri, 29 Dec 2023 19:21:43 +0100 Subject: [PATCH 3/3] Only lint ranges that really overlap --- compiler/rustc_pattern_analysis/src/lib.rs | 4 +- compiler/rustc_pattern_analysis/src/lints.rs | 87 +------------ .../rustc_pattern_analysis/src/usefulness.rs | 117 +++++++++++++++++- ...rlapping_range_endpoints-false-positive.rs | 9 ++ .../overlapping_range_endpoints.rs | 11 +- .../overlapping_range_endpoints.stderr | 24 +--- 6 files changed, 137 insertions(+), 115 deletions(-) create mode 100644 tests/ui/pattern/usefulness/integer-ranges/issue-117648-overlapping_range_endpoints-false-positive.rs diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 5d6a0f855f5..374914055d8 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -125,7 +125,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. diff --git a/compiler/rustc_pattern_analysis/src/lints.rs b/compiler/rustc_pattern_analysis/src/lints.rs index 941bdad1fd9..cfe4ca3ce93 100644 --- a/compiler/rustc_pattern_analysis/src/lints.rs +++ b/compiler/rustc_pattern_analysis/src/lints.rs @@ -1,12 +1,7 @@ -use smallvec::SmallVec; - -use rustc_data_structures::captures::Captures; -use rustc_middle::ty; use rustc_session::lint; use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS; use rustc_span::ErrorGuaranteed; -use crate::constructor::MaybeInfiniteInt; use crate::errors::{ self, NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered, }; @@ -15,7 +10,6 @@ self, Constructor, DeconstructedPat, MatchArm, MatchCtxt, PlaceCtxt, RevealedTy, RustcMatchCheckCtxt, SplitConstructorSet, WitnessPat, }; -use crate::usefulness::OverlappingRanges; /// A column of patterns in the matrix, where a column is the intuitive notion of "subpatterns that /// inspect the same subvalue/place". @@ -68,10 +62,6 @@ fn analyze_ctors( Ok(ctors_for_ty.split(pcx, column_ctors)) } - fn iter(&self) -> impl Iterator> + 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 @@ -207,82 +197,10 @@ pub(crate) fn lint_nonexhaustive_missing_variants<'a, 'p, 'tcx>( Ok(()) } -/// 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>, -) -> Result<(), ErrorGuaranteed> { - let Some(ty) = column.head_ty() else { - return Ok(()); - }; - 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)?; - } - } - } - Ok(()) -} - -#[instrument(level = "debug", skip(cx))] pub(crate) fn lint_overlapping_range_endpoints<'a, 'p, 'tcx>( cx: MatchCtxt<'a, 'p, 'tcx>, - column: &PatternColumn<'p, 'tcx>, -) -> Result<(), ErrorGuaranteed> { - let mut overlapping_range_endpoints = Vec::new(); - collect_overlapping_range_endpoints(cx, column, &mut overlapping_range_endpoints)?; - + overlapping_range_endpoints: &[rustc::OverlappingRanges<'p, 'tcx>], +) { let rcx = cx.tycx; for overlap in overlapping_range_endpoints { let overlap_as_pat = rcx.hoist_pat_range(&overlap.overlaps_on, overlap.pat.ty()); @@ -300,5 +218,4 @@ pub(crate) fn lint_overlapping_range_endpoints<'a, 'p, 'tcx>( errors::OverlappingRangeEndpoints { overlap: overlaps, range: pat_span }, ); } - Ok(()) } diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 426c0e8aa86..85b6a6a3b6c 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -1328,6 +1328,83 @@ fn extend(&mut self, other: Self) { } } +/// Collect ranges that overlap like `lo..=overlap`/`overlap..=hi`. Must be called during +/// exhaustiveness checking, if we find a singleton range after constructor splitting. This reuses +/// row intersection information to only detect ranges that truly overlap. +/// +/// If two ranges overlapped, the split set will contain their intersection as a singleton. +/// Specialization will then select rows that match the overlap, and exhaustiveness will compute +/// which rows have an intersection that includes the overlap. That gives us all the info we need to +/// compute overlapping ranges without false positives. +/// +/// We can however get false negatives because exhaustiveness does not explore all cases. See the +/// section on relevancy at the top of the file. +fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>( + overlap_range: IntRange, + matrix: &Matrix<'p, Cx>, + specialized_matrix: &Matrix<'p, Cx>, + overlapping_range_endpoints: &mut Vec>, +) { + 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 `specialized_matrix` which + // contains only rows that matched the current `ctor` as well as accurate intersection + // information. It doesn't contain the column that contains the range; that can be found in + // `matrix`. + for (child_row_id, child_row) in specialized_matrix.rows().enumerate() { + let PatOrWild::Pat(pat) = matrix.rows[child_row.parent_row].head() else { continue }; + 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)) + } + } +} + /// The core of the algorithm. /// /// This recursively computes witnesses of the non-exhaustiveness of `matrix` (if any). Also tracks @@ -1346,6 +1423,7 @@ fn extend(&mut self, other: Self) { fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( mcx: MatchCtxt<'a, 'p, Cx>, matrix: &mut Matrix<'p, Cx>, + overlapping_range_endpoints: &mut Vec>, is_top_level: bool, ) -> Result, Cx::Error> { debug_assert!(matrix.rows().all(|r| r.len() == matrix.column_count())); @@ -1425,7 +1503,12 @@ 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`. @@ -1447,6 +1530,21 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( } } } + + // Detect ranges that overlap on their endpoints. + if let Constructor::IntRange(overlap_range) = ctor { + if overlap_range.is_singleton() + && spec_matrix.rows.len() >= 2 + && spec_matrix.rows.iter().any(|row| !row.intersects.is_empty()) + { + collect_overlapping_range_endpoints( + overlap_range, + matrix, + &spec_matrix, + overlapping_range_endpoints, + ); + } + } } // Record usefulness in the patterns. @@ -1487,6 +1585,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>, + pub overlapping_range_endpoints: Vec>, } /// Computes whether a match is exhaustive and which of its arms are useful. @@ -1497,9 +1596,14 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>( scrut_ty: Cx::Ty, scrut_validity: ValidityConstraint, ) -> Result, Cx::Error> { + let mut overlapping_range_endpoints = Vec::new(); let mut matrix = Matrix::new(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 @@ -1516,5 +1620,10 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>( (arm, usefulness) }) .collect(); - Ok(UsefulnessReport { arm_usefulness, non_exhaustiveness_witnesses }) + + Ok(UsefulnessReport { + arm_usefulness, + non_exhaustiveness_witnesses, + overlapping_range_endpoints, + }) } diff --git a/tests/ui/pattern/usefulness/integer-ranges/issue-117648-overlapping_range_endpoints-false-positive.rs b/tests/ui/pattern/usefulness/integer-ranges/issue-117648-overlapping_range_endpoints-false-positive.rs new file mode 100644 index 00000000000..37fcb4b4af9 --- /dev/null +++ b/tests/ui/pattern/usefulness/integer-ranges/issue-117648-overlapping_range_endpoints-false-positive.rs @@ -0,0 +1,9 @@ +// check-pass +fn main() { + match (0i8, 0i8) { + (0, _) => {} + (..=-1, ..=0) => {} + (1.., 0..) => {} + (1.., ..=-1) | (..=-1, 1..) => {} + } +} diff --git a/tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs b/tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs index 33c1dfd39d4..7e56880a87f 100644 --- a/tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs +++ b/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) { @@ -58,4 +58,11 @@ fn main() { Some(10..20) => {} //~ ERROR multiple patterns overlap on their endpoints _ => {} } + + // The lint has false negatives when we skip some cases because of relevancy. + match (true, true, 0u8) { + (true, _, 0..=10) => {} + (_, true, 10..20) => {} + _ => {} + } } diff --git a/tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr b/tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr index a87205d76d1..aa37bd9bc9c 100644 --- a/tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr +++ b/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