Skip to content

Commit

Permalink
Extract the two annoying lints into their own functions
Browse files Browse the repository at this point in the history
  • Loading branch information
@Nadrieril
Nadrieril committed Sep 21, 2023
1 parent 7925e12 commit b5728f1
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 108 deletions.
87 changes: 24 additions & 63 deletions compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,13 @@ use smallvec::SmallVec;

use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::{HirId, RangeEnd};
use rustc_hir::RangeEnd;
use rustc_index::Idx;
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::mir;
use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange};
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::{self, Ty, TyCtxt, VariantDef};
use rustc_session::lint;
use rustc_span::{Span, DUMMY_SP};
use rustc_target::abi::{FieldIdx, Integer, Size, VariantIdx, FIRST_VARIANT};

Expand All @@ -68,7 +67,6 @@ use self::SliceKind::*;

use super::compare_const_vals;
use super::usefulness::{MatchCheckCtxt, PatCtxt};
use crate::errors::{Overlap, OverlappingRangeEndpoints};

/// Recursively expand this pattern into its subpatterns. Only useful for or-patterns.
fn expand_or_pat<'p, 'tcx>(pat: &'p Pat<'tcx>) -> Vec<&'p Pat<'tcx>> {
Expand Down Expand Up @@ -112,7 +110,7 @@ impl IntRange {
self.range.start() == self.range.end()
}

fn boundaries(&self) -> (u128, u128) {
pub(super) fn boundaries(&self) -> (u128, u128) {
(*self.range.start(), *self.range.end())
}

Expand Down Expand Up @@ -188,7 +186,7 @@ impl IntRange {
}

/// Only used for displaying the range properly.
fn to_pat<'tcx>(&self, tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Pat<'tcx> {
pub(super) fn to_pat<'tcx>(&self, tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Pat<'tcx> {
let (lo, hi) = self.boundaries();

let bias = IntRange::signed_bias(tcx, ty);
Expand All @@ -211,69 +209,36 @@ impl IntRange {
Pat { ty, span: DUMMY_SP, kind }
}

/// Lint on likely incorrect range patterns (#63987)
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>, bool, HirId)>,
column_count: usize,
) {
// 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;
}

let overlap: u128 = self.boundaries().0;
// Spans of ranges that start or end with the overlap.
/// Given a list of ranges, gather all the ranges that have `endpoint` as an endpoint, and match
/// them with ranges that came before and had `endpoint` as their other endpoint.
/// In other words, for each range we gather the ranges above it that it intersects exactly on
/// `endpoint`.
pub(super) fn gather_endpoint_overlaps<'a, T: Clone>(
endpoint: u128,
ranges: impl Iterator<Item = (&'a IntRange, T)>,
) -> Vec<(T, SmallVec<[T; 1]>)> {
// Spans of ranges that start or end with the endpoint.
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, arm_hir_id) in
pats.filter_map(|(pat, under_guard, arm_hir_id)| {
Some((pat.ctor().as_int_range()?, pat.span(), under_guard, arm_hir_id))
})
{
let mut gathered_overlaps: Vec<_> = Vec::new();
// Iterate on rows that contained `endpoint` as an endpoint.
for (range, arm_data) in ranges {
if range.is_singleton() {
continue;
}
let mut overlaps: SmallVec<[_; 1]> = Default::default();
if *range.range.start() == overlap {
if *range.range.start() == endpoint {
if !prefixes.is_empty() {
overlaps = prefixes.clone();
gathered_overlaps.push((arm_data.clone(), prefixes.clone()));
}
if !is_under_guard {
suffixes.push(this_span)
}
} else if *range.range.end() == overlap {
suffixes.push(arm_data)
} else if *range.range.end() == endpoint {
if !suffixes.is_empty() {
overlaps = suffixes.clone();
}
if !is_under_guard {
prefixes.push(this_span)
gathered_overlaps.push((arm_data.clone(), suffixes.clone()));
}
}
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,
arm_hir_id,
this_span,
OverlappingRangeEndpoints { overlap: overlaps, range: this_span },
);
prefixes.push(arm_data)
}
}
gathered_overlaps
}
}

Expand Down Expand Up @@ -658,21 +623,17 @@ pub(super) enum Constructor<'tcx> {
}

impl<'tcx> Constructor<'tcx> {
pub(super) fn is_wildcard(&self) -> bool {
matches!(self, Wildcard)
}

pub(super) fn is_non_exhaustive(&self) -> bool {
matches!(self, NonExhaustive)
}

fn as_variant(&self) -> Option<VariantIdx> {
pub(super) fn as_variant(&self) -> Option<VariantIdx> {
match self {
Variant(i) => Some(*i),
_ => None,
}
}
fn as_int_range(&self) -> Option<&IntRange> {
pub(super) fn as_int_range(&self) -> Option<&IntRange> {
match self {
IntRange(range) => Some(range),
_ => None,
Expand Down
130 changes: 85 additions & 45 deletions compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,16 +305,16 @@
//! stay as a full constant and become an `Opaque` pattern. These `Opaque` patterns do not participate
//! in exhaustiveness, specialization or overlap checking.

use super::deconstruct_pat::{Constructor, ConstructorSet, DeconstructedPat, Fields};
use crate::errors::{NonExhaustiveOmittedPattern, Uncovered};
use super::deconstruct_pat::{Constructor, ConstructorSet, DeconstructedPat, Fields, IntRange};
use crate::errors::{NonExhaustiveOmittedPattern, Overlap, OverlappingRangeEndpoints, Uncovered};

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;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS;
use rustc_session::lint::builtin::{NON_EXHAUSTIVE_OMITTED_PATTERNS, OVERLAPPING_RANGE_ENDPOINTS};
use rustc_span::{Span, DUMMY_SP};

use smallvec::{smallvec, SmallVec};
Expand Down Expand Up @@ -732,6 +732,68 @@ impl<'p, 'tcx> WitnessMatrix<'p, 'tcx> {
}
}

fn lint_overlapping_range_endpoints<'a, 'p: 'a, 'tcx: 'a>(
pcx: &PatCtxt<'_, 'p, 'tcx>,
overlap_range: &IntRange,
rows: impl Iterator<Item = &'a PatStack<'p, 'tcx>>,
) {
let endpoint: u128 = overlap_range.boundaries().0;
let rows = rows.filter_map(|row| Some((row.head().ctor().as_int_range()?, row)));
let gathered_overlaps = IntRange::gather_endpoint_overlaps(endpoint, rows);
if !gathered_overlaps.is_empty() {
let overlap_as_pat = overlap_range.to_pat(pcx.cx.tcx, pcx.ty);
for (base_row, overlaps) in gathered_overlaps {
let overlaps: Vec<_> = overlaps
.into_iter()
.filter(|overlapped_row| !overlapped_row.is_under_guard)
.map(|overlapped_row| Overlap {
range: overlap_as_pat.clone(),
span: overlapped_row.head().span(),
})
.collect();
if !overlaps.is_empty() {
let arm_span = base_row.head().span();
pcx.cx.tcx.emit_spanned_lint(
OVERLAPPING_RANGE_ENDPOINTS,
base_row.arm_hir_id,
arm_span,
OverlappingRangeEndpoints { overlap: overlaps, range: arm_span },
);
}
}
}
}

fn lint_non_exhaustive_omitted_patterns<'a, 'p: 'a, 'tcx: 'a>(
pcx: &PatCtxt<'_, 'p, 'tcx>,
missing_ctors: &[Constructor<'tcx>],
lint_row: &'a PatStack<'p, 'tcx>,
) {
let patterns = missing_ctors
.iter()
// We only list real variants.
.filter(|c| c.as_variant().is_some())
.cloned()
.map(|missing_ctor| DeconstructedPat::wild_from_ctor(pcx, missing_ctor))
.collect::<Vec<_>>();

if !patterns.is_empty() {
let wildcard_span = lint_row.head().span();
// Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns`
// is not exhaustive enough.
// NB: The partner lint for structs lives in `compiler/rustc_hir_analysis/src/check/pat.rs`.
pcx.cx.tcx.emit_spanned_lint(
NON_EXHAUSTIVE_OMITTED_PATTERNS,
lint_row.arm_hir_id,
wildcard_span,
NonExhaustiveOmittedPattern {
scrut_ty: pcx.ty,
uncovered: Uncovered::new(wildcard_span, pcx.cx, patterns),
},
);
}
}

/// Algorithm from <http://moscova.inria.fr/~maranget/papers/warn/index.html>.
/// The algorithm from the paper has been modified to correctly handle empty
/// types. The changes are:
Expand Down Expand Up @@ -812,63 +874,41 @@ 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)
// Lint on likely incorrect range patterns (#63987).
if spec_matrix.rows().len() >= 2 && matches!(ctor_set, ConstructorSet::Integers { .. }) {
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`.
//
// FIXME: for now, only check for overlapping ranges on non-nested (column_count ==
// 1) range patterns. Otherwise with the current logic the following is detected as
// overlapping:
// ```
// match (0u8, true) {
// (0 ..= 125, false) => {}
// (125 ..= 255, true) => {}
// _ => {}
// }
// ```
if overlap_range.is_singleton() && matrix.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,
parent_row.arm_hir_id,
)
},
),
matrix.column_count(),
);
let parent_rows =
spec_matrix.rows().map(|child_row| &matrix.rows[child_row.parent_row]);
lint_overlapping_range_endpoints(pcx, overlap_range, parent_rows);
}
}
}

// When all the conditions are met we have a match with a `non_exhaustive` enum
// that has the potential to trigger the `non_exhaustive_omitted_patterns` lint.
// Lint on incomplete non_exhaustive match (#89554).
if cx.refutable
&& (matches!(&ctor, Constructor::Missing)
|| (matches!(&ctor, Constructor::Wildcard) && is_top_level))
&& matches!(&ctor_set, ConstructorSet::Variants { non_exhaustive: true, .. })
&& spec_matrix.rows().len() != 0
{
let patterns = missing_ctors
.iter()
// We only list real variants.
.filter(|c| !(c.is_non_exhaustive() || c.is_wildcard()))
.cloned()
.map(|missing_ctor| DeconstructedPat::wild_from_ctor(pcx, missing_ctor))
.collect::<Vec<_>>();

if !patterns.is_empty() {
let first_spec_row = spec_matrix.rows().next().unwrap();
let first_wildcard_row = &matrix.rows[first_spec_row.parent_row];
let wildcard_span = first_wildcard_row.head().span();
// Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns`
// is not exhaustive enough.
// NB: The partner lint for structs lives in `compiler/rustc_hir_analysis/src/check/pat.rs`.
cx.tcx.emit_spanned_lint(
NON_EXHAUSTIVE_OMITTED_PATTERNS,
first_wildcard_row.arm_hir_id,
wildcard_span,
NonExhaustiveOmittedPattern {
scrut_ty: pcx.ty,
uncovered: Uncovered::new(wildcard_span, pcx.cx, patterns),
},
);
}
let mut parent_rows =
spec_matrix.rows().map(|child_row| &matrix.rows[child_row.parent_row]);
let first_parent_row = parent_rows.next().unwrap();
lint_non_exhaustive_omitted_patterns(pcx, &missing_ctors, first_parent_row);
}

for child_row in spec_matrix.rows() {
Expand Down

0 comments on commit b5728f1

Please sign in to comment.