Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint overlapping_ranges_endpoints directly instead of collecting into a Vec #120002

Merged
merged 2 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions compiler/rustc_pattern_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,9 @@ use rustc_middle::ty::Ty;
#[cfg(feature = "rustc")]
use rustc_span::ErrorGuaranteed;

use crate::constructor::{Constructor, ConstructorSet};
use crate::constructor::{Constructor, ConstructorSet, IntRange};
#[cfg(feature = "rustc")]
use crate::lints::{
lint_nonexhaustive_missing_variants, lint_overlapping_range_endpoints, PatternColumn,
};
use crate::lints::{lint_nonexhaustive_missing_variants, PatternColumn};
use crate::pat::DeconstructedPat;
#[cfg(feature = "rustc")]
use crate::rustc::RustcMatchCheckCtxt;
Expand Down Expand Up @@ -77,6 +75,17 @@ pub trait TypeCx: Sized + fmt::Debug {

/// Raise a bug.
fn bug(&self, fmt: fmt::Arguments<'_>) -> !;

/// Lint 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.
/// The default implementation does nothing.
fn lint_overlapping_range_endpoints(
&self,
_pat: &DeconstructedPat<'_, Self>,
_overlaps_on: IntRange,
_overlaps_with: &[&DeconstructedPat<'_, Self>],
) {
}
}

/// Context that provides information global to a match.
Expand Down Expand Up @@ -111,16 +120,10 @@ pub fn analyze_match<'p, 'tcx>(

let report = compute_match_usefulness(cx, arms, scrut_ty, scrut_validity)?;

let pat_column = PatternColumn::new(arms);

// Lint ranges that overlap on their endpoints, which is likely a mistake.
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.
if tycx.refutable && report.non_exhaustiveness_witnesses.is_empty() {
let pat_column = PatternColumn::new(arms);
lint_nonexhaustive_missing_variants(cx, arms, &pat_column, scrut_ty)?;
}

Expand Down
32 changes: 3 additions & 29 deletions compiler/rustc_pattern_analysis/src/lints.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
use rustc_session::lint;
use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS;
use rustc_span::ErrorGuaranteed;

use crate::errors::{
self, NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered,
};
use crate::errors::{NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered};
use crate::pat::PatOrWild;
use crate::rustc::{
self, Constructor, DeconstructedPat, MatchArm, MatchCtxt, PlaceCtxt, RevealedTy,
RustcMatchCheckCtxt, SplitConstructorSet, WitnessPat,
Constructor, DeconstructedPat, MatchArm, MatchCtxt, PlaceCtxt, RevealedTy, RustcMatchCheckCtxt,
SplitConstructorSet, WitnessPat,
};

/// A column of patterns in the matrix, where a column is the intuitive notion of "subpatterns that
Expand Down Expand Up @@ -196,26 +193,3 @@ pub(crate) fn lint_nonexhaustive_missing_variants<'a, 'p, 'tcx>(
}
Ok(())
}

pub(crate) fn lint_overlapping_range_endpoints<'a, 'p, 'tcx>(
cx: MatchCtxt<'a, 'p, 'tcx>,
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());
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 },
);
}
}
37 changes: 27 additions & 10 deletions compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,26 @@
use smallvec::SmallVec;
use std::fmt;
use std::iter::once;

use rustc_arena::{DroplessArena, TypedArena};
use rustc_data_structures::captures::Captures;
use rustc_hir::def_id::DefId;
use rustc_hir::HirId;
use rustc_index::Idx;
use rustc_index::IndexVec;
use rustc_index::{Idx, IndexVec};
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::{self, Const};
use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange, PatRangeBoundary};
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::TypeVisitableExt;
use rustc_middle::ty::{self, OpaqueTypeKey, Ty, TyCtxt, VariantDef};
use rustc_span::ErrorGuaranteed;
use rustc_span::{Span, DUMMY_SP};
use rustc_middle::ty::{self, OpaqueTypeKey, Ty, TyCtxt, TypeVisitableExt, VariantDef};
use rustc_session::lint;
use rustc_span::{ErrorGuaranteed, Span, DUMMY_SP};
use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT};
use smallvec::SmallVec;

use crate::constructor::{
IntRange, MaybeInfiniteInt, OpaqueId, RangeEnd, Slice, SliceKind, VariantVisibility,
};
use crate::TypeCx;
use crate::{errors, TypeCx};

use crate::constructor::Constructor::*;

Expand All @@ -34,8 +32,6 @@ pub type DeconstructedPat<'p, 'tcx> =
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, 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, RustcMatchCheckCtxt<'p, 'tcx>>;
pub(crate) type SplitConstructorSet<'p, 'tcx> =
Expand Down Expand Up @@ -991,6 +987,27 @@ impl<'p, 'tcx> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> {
fn bug(&self, fmt: fmt::Arguments<'_>) -> ! {
span_bug!(self.scrut_span, "{}", fmt)
}

fn lint_overlapping_range_endpoints(
&self,
pat: &crate::pat::DeconstructedPat<'_, Self>,
overlaps_on: IntRange,
overlaps_with: &[&crate::pat::DeconstructedPat<'_, Self>],
) {
let overlap_as_pat = self.hoist_pat_range(&overlaps_on, pat.ty());
let overlaps: Vec<_> = overlaps_with
.iter()
.map(|pat| pat.data().unwrap().span)
.map(|span| errors::Overlap { range: overlap_as_pat.clone(), span })
.collect();
let pat_span = pat.data().unwrap().span;
self.tcx.emit_spanned_lint(
lint::builtin::OVERLAPPING_RANGE_ENDPOINTS,
self.match_lint_level,
pat_span,
errors::OverlappingRangeEndpoints { overlap: overlaps, range: pat_span },
);
}
}

/// Recursively expand this pattern into its subpatterns. Only useful for or-patterns.
Expand Down
54 changes: 8 additions & 46 deletions compiler/rustc_pattern_analysis/src/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1340,10 +1340,10 @@ impl<Cx: TypeCx> WitnessMatrix<Cx> {
/// 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>(
mcx: MatchCtxt<'_, Cx>,
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`.
Expand Down Expand Up @@ -1373,11 +1373,7 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
.map(|&(_, pat)| pat)
.collect();
if !overlaps_with.is_empty() {
overlapping_range_endpoints.push(OverlappingRanges {
pat,
overlaps_on: overlap_range,
overlaps_with,
});
mcx.tycx.lint_overlapping_range_endpoints(pat, overlap_range, &overlaps_with);
}
}
suffixes.push((child_row_id, pat))
Expand All @@ -1393,11 +1389,7 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
.map(|&(_, pat)| pat)
.collect();
if !overlaps_with.is_empty() {
overlapping_range_endpoints.push(OverlappingRanges {
pat,
overlaps_on: overlap_range,
overlaps_with,
});
mcx.tycx.lint_overlapping_range_endpoints(pat, overlap_range, &overlaps_with);
}
}
prefixes.push((child_row_id, pat))
Expand All @@ -1423,7 +1415,6 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>(
fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
mcx: MatchCtxt<'a, 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()));
Expand Down Expand Up @@ -1503,12 +1494,7 @@ 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,
overlapping_range_endpoints,
false,
)
compute_exhaustiveness_and_usefulness(mcx, &mut spec_matrix, false)
})?;

// Transform witnesses for `spec_matrix` into witnesses for `matrix`.
Expand Down Expand Up @@ -1537,12 +1523,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
&& 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,
);
collect_overlapping_range_endpoints(mcx, overlap_range, matrix, &spec_matrix);
}
}
}
Expand All @@ -1569,23 +1550,13 @@ 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.
pub arm_usefulness: Vec<(MatchArm<'p, Cx>, Usefulness<'p, Cx>)>,
/// 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.
Expand All @@ -1596,14 +1567,9 @@ 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,
&mut overlapping_range_endpoints,
true,
)?;
let non_exhaustiveness_witnesses =
compute_exhaustiveness_and_usefulness(cx, &mut matrix, true)?;

let non_exhaustiveness_witnesses: Vec<_> = non_exhaustiveness_witnesses.single_column();
let arm_usefulness: Vec<_> = arms
Expand All @@ -1621,9 +1587,5 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>(
})
.collect();

Ok(UsefulnessReport {
arm_usefulness,
non_exhaustiveness_witnesses,
overlapping_range_endpoints,
})
Ok(UsefulnessReport { arm_usefulness, non_exhaustiveness_witnesses })
}
Loading