mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Only lint ranges that really overlap

Browse Source
This commit is contained in:
Nadrieril 2023-12-29 19:21:43 +01:00
parent 6f6ba2571d
commit a24f4db41b
6 changed files with 137 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
compiler/rustc_pattern_analysis/src/lib.rs
View File

@ -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.

87
compiler/rustc_pattern_analysis/src/lints.rs
View File

@ -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<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
@ -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<rustc::OverlappingRanges<'p, 'tcx>>,
) -> 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(())
}

117
compiler/rustc_pattern_analysis/src/usefulness.rs
View File

@ -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<OverlappingRanges<'p, Cx>>,
) {
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<OverlappingRanges<'p, Cx>>,
is_top_level: bool,
) -> Result<WitnessMatrix<Cx>, 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<WitnessPat<Cx>>,
pub overlapping_range_endpoints: Vec<OverlappingRanges<'p, Cx>>,
}
/// 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<UsefulnessReport<'p, Cx>, 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,
})
}

9
tests/ui/pattern/usefulness/integer-ranges/issue-117648-overlapping_range_endpoints-false-positive.rs Normal file
View File

@ -0,0 +1,9 @@
// check-pass
fn main() {
match (0i8, 0i8) {
(0, _) => {}
(..=-1, ..=0) => {}
(1.., 0..) => {}
(1.., ..=-1) | (..=-1, 1..) => {}
}
}

11
tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs
View File

@ -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) => {}
_ => {}
}
}

24
tests/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr
View File

@ -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