diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 7350f89018be2..02b391615b621 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -989,6 +989,15 @@ pub enum RangeEnd { Excluded, } +impl fmt::Display for RangeEnd { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + RangeEnd::Included => "..=", + RangeEnd::Excluded => "..", + }) + } +} + #[derive(RustcEncodable, RustcDecodable, Debug, HashStable)] pub enum PatKind { /// Represents a wildcard pattern (i.e., `_`). diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 983e3a9922ec2..4c28f6372fe2c 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -80,6 +80,12 @@ declare_lint! { "detects unreachable patterns" } +declare_lint! { + pub OVERLAPPING_PATTERNS, + Warn, + "detects overlapping patterns" +} + declare_lint! { pub UNUSED_MACROS, Warn, @@ -423,6 +429,7 @@ declare_lint_pass! { DEAD_CODE, UNREACHABLE_CODE, UNREACHABLE_PATTERNS, + OVERLAPPING_PATTERNS, UNUSED_MACROS, WARNINGS, UNUSED_FEATURES, diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 0e054013cd779..e3860e229d6b5 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -255,6 +255,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { UNUSED_MUT, UNREACHABLE_CODE, UNREACHABLE_PATTERNS, + OVERLAPPING_PATTERNS, UNUSED_MUST_USE, UNUSED_UNSAFE, PATH_STATEMENTS, diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 3ea5805287724..1d83b104177e2 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -167,13 +167,14 @@ use super::{FieldPat, Pat, PatKind, PatRange}; use super::{PatternFoldable, PatternFolder, compare_const_vals}; use rustc::hir::def_id::DefId; -use rustc::hir::RangeEnd; +use rustc::hir::{RangeEnd, HirId}; use rustc::ty::{self, Ty, TyCtxt, TypeFoldable, Const}; use rustc::ty::layout::{Integer, IntegerExt, VariantIdx, Size}; use rustc::mir::Field; use rustc::mir::interpret::{ConstValue, Scalar, truncate, AllocId, Pointer}; use rustc::util::common::ErrorReported; +use rustc::lint; use syntax::attr::{SignedInt, UnsignedInt}; use syntax_pos::{Span, DUMMY_SP}; @@ -418,7 +419,7 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { } } -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug)] enum Constructor<'tcx> { /// The constructor of all patterns that don't vary by constructor, /// e.g., struct patterns and fixed-length arrays. @@ -426,13 +427,30 @@ enum Constructor<'tcx> { /// Enum variants. Variant(DefId), /// Literal values. - ConstantValue(&'tcx ty::Const<'tcx>), + ConstantValue(&'tcx ty::Const<'tcx>, Span), /// Ranges of literal values (`2..=5` and `2..5`). - ConstantRange(u128, u128, Ty<'tcx>, RangeEnd), + ConstantRange(u128, u128, Ty<'tcx>, RangeEnd, Span), /// Array patterns of length n. Slice(u64), } +// Ignore spans when comparing, they don't carry semantic information as they are only for lints. +impl<'tcx> std::cmp::PartialEq for Constructor<'tcx> { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Constructor::Single, Constructor::Single) => true, + (Constructor::Variant(a), Constructor::Variant(b)) => a == b, + (Constructor::ConstantValue(a, _), Constructor::ConstantValue(b, _)) => a == b, + ( + Constructor::ConstantRange(a_start, a_end, a_ty, a_range_end, _), + Constructor::ConstantRange(b_start, b_end, b_ty, b_range_end, _), + ) => a_start == b_start && a_end == b_end && a_ty == b_ty && a_range_end == b_range_end, + (Constructor::Slice(a), Constructor::Slice(b)) => a == b, + _ => false, + } + } +} + impl<'tcx> Constructor<'tcx> { fn is_slice(&self) -> bool { match self { @@ -447,15 +465,33 @@ impl<'tcx> Constructor<'tcx> { adt: &'tcx ty::AdtDef, ) -> VariantIdx { match self { - &Variant(id) => adt.variant_index_with_id(id), - &Single => { + Variant(id) => adt.variant_index_with_id(*id), + Single => { assert!(!adt.is_enum()); VariantIdx::new(0) } - &ConstantValue(c) => crate::const_eval::const_variant_index(cx.tcx, cx.param_env, c), + ConstantValue(c, _) => crate::const_eval::const_variant_index(cx.tcx, cx.param_env, c), _ => bug!("bad constructor {:?} for adt {:?}", self, adt) } } + + fn display(&self, tcx: TyCtxt<'tcx>) -> String { + match self { + Constructor::ConstantValue(val, _) => format!("{}", val), + Constructor::ConstantRange(lo, hi, ty, range_end, _) => { + // Get the right sign on the output: + let ty = ty::ParamEnv::empty().and(*ty); + format!( + "{}{}{}", + ty::Const::from_bits(tcx, *lo, ty), + range_end, + ty::Const::from_bits(tcx, *hi, ty), + ) + } + Constructor::Slice(val) => format!("[{}]", val), + _ => bug!("bad constructor being displayed: `{:?}", self), + } + } } #[derive(Clone, Debug)] @@ -484,6 +520,7 @@ pub enum WitnessPreference { struct PatCtxt<'tcx> { ty: Ty<'tcx>, max_slice_length: u64, + span: Span, } /// A witness of non-exhaustiveness for error reporting, represented @@ -610,8 +647,8 @@ impl<'tcx> Witness<'tcx> { _ => { match *ctor { - ConstantValue(value) => PatKind::Constant { value }, - ConstantRange(lo, hi, ty, end) => PatKind::Range(PatRange { + ConstantValue(value, _) => PatKind::Constant { value }, + ConstantRange(lo, hi, ty, end, _) => PatKind::Range(PatRange { lo: ty::Const::from_bits(cx.tcx, lo, ty::ParamEnv::empty().and(ty)), hi: ty::Const::from_bits(cx.tcx, hi, ty::ParamEnv::empty().and(ty)), end, @@ -647,7 +684,7 @@ fn all_constructors<'a, 'tcx>( let ctors = match pcx.ty.kind { ty::Bool => { [true, false].iter().map(|&b| { - ConstantValue(ty::Const::from_bool(cx.tcx, b)) + ConstantValue(ty::Const::from_bool(cx.tcx, b), pcx.span) }).collect() } ty::Array(ref sub_ty, len) if len.try_eval_usize(cx.tcx, cx.param_env).is_some() => { @@ -679,15 +716,19 @@ fn all_constructors<'a, 'tcx>( ty::Char => { vec![ // The valid Unicode Scalar Value ranges. - ConstantRange('\u{0000}' as u128, - '\u{D7FF}' as u128, - cx.tcx.types.char, - RangeEnd::Included + ConstantRange( + '\u{0000}' as u128, + '\u{D7FF}' as u128, + cx.tcx.types.char, + RangeEnd::Included, + pcx.span, ), - ConstantRange('\u{E000}' as u128, - '\u{10FFFF}' as u128, - cx.tcx.types.char, - RangeEnd::Included + ConstantRange( + '\u{E000}' as u128, + '\u{10FFFF}' as u128, + cx.tcx.types.char, + RangeEnd::Included, + pcx.span, ), ] } @@ -695,12 +736,12 @@ fn all_constructors<'a, 'tcx>( let bits = Integer::from_attr(&cx.tcx, SignedInt(ity)).size().bits() as u128; let min = 1u128 << (bits - 1); let max = min - 1; - vec![ConstantRange(min, max, pcx.ty, RangeEnd::Included)] + vec![ConstantRange(min, max, pcx.ty, RangeEnd::Included, pcx.span)] } ty::Uint(uty) => { let size = Integer::from_attr(&cx.tcx, UnsignedInt(uty)).size(); let max = truncate(u128::max_value(), size); - vec![ConstantRange(0, max, pcx.ty, RangeEnd::Included)] + vec![ConstantRange(0, max, pcx.ty, RangeEnd::Included, pcx.span)] } _ => { if cx.is_uninhabited(pcx.ty) { @@ -827,10 +868,11 @@ where /// /// `IntRange` is never used to encode an empty range or a "range" that wraps /// around the (offset) space: i.e., `range.lo <= range.hi`. -#[derive(Clone)] +#[derive(Clone, Debug)] struct IntRange<'tcx> { pub range: RangeInclusive, pub ty: Ty<'tcx>, + pub span: Span, } impl<'tcx> IntRange<'tcx> { @@ -860,6 +902,7 @@ impl<'tcx> IntRange<'tcx> { tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, value: &Const<'tcx>, + span: Span, ) -> Option> { if let Some((target_size, bias)) = Self::integral_size_and_signed_bias(tcx, value.ty) { let ty = value.ty; @@ -877,7 +920,7 @@ impl<'tcx> IntRange<'tcx> { return None }; let val = val ^ bias; - Some(IntRange { range: val..=val, ty }) + Some(IntRange { range: val..=val, ty, span }) } else { None } @@ -890,6 +933,7 @@ impl<'tcx> IntRange<'tcx> { hi: u128, ty: Ty<'tcx>, end: &RangeEnd, + span: Span, ) -> Option> { if Self::is_integral(ty) { // Perform a shift if the underlying types are signed, @@ -901,7 +945,7 @@ impl<'tcx> IntRange<'tcx> { None } else { let offset = (*end == RangeEnd::Excluded) as u128; - Some(IntRange { range: lo..=(hi - offset), ty }) + Some(IntRange { range: lo..=(hi - offset), ty, span }) } } else { None @@ -916,8 +960,8 @@ impl<'tcx> IntRange<'tcx> { // Floating-point ranges are permitted and we don't want // to consider them when constructing integer ranges. match ctor { - ConstantRange(lo, hi, ty, end) => Self::from_range(tcx, *lo, *hi, ty, end), - ConstantValue(val) => Self::from_const(tcx, param_env, val), + ConstantRange(lo, hi, ty, end, span) => Self::from_range(tcx, *lo, *hi, ty, end, *span), + ConstantValue(val, span) => Self::from_const(tcx, param_env, val, *span), _ => None, } } @@ -930,7 +974,7 @@ impl<'tcx> IntRange<'tcx> { loop { match pat.kind { box PatKind::Constant { value } => { - return Self::from_const(tcx, param_env, value); + return Self::from_const(tcx, param_env, value, pat.span); } box PatKind::Range(PatRange { lo, hi, end }) => { return Self::from_range( @@ -939,6 +983,7 @@ impl<'tcx> IntRange<'tcx> { hi.eval_bits(tcx, param_env, hi.ty), &lo.ty, &end, + pat.span, ); } box PatKind::AscribeUserType { ref subpattern, .. } => { @@ -965,14 +1010,15 @@ impl<'tcx> IntRange<'tcx> { tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, r: RangeInclusive, + span: Span, ) -> Constructor<'tcx> { let bias = IntRange::signed_bias(tcx, ty); let (lo, hi) = r.into_inner(); if lo == hi { let ty = ty::ParamEnv::empty().and(ty); - ConstantValue(ty::Const::from_bits(tcx, lo ^ bias, ty)) + ConstantValue(ty::Const::from_bits(tcx, lo ^ bias, ty), span) } else { - ConstantRange(lo ^ bias, hi ^ bias, ty, RangeEnd::Included) + ConstantRange(lo ^ bias, hi ^ bias, ty, RangeEnd::Included, span) } } @@ -995,17 +1041,23 @@ impl<'tcx> IntRange<'tcx> { if lo > subrange_hi || subrange_lo > hi { // The pattern doesn't intersect with the subrange at all, // so the subrange remains untouched. - remaining_ranges.push(Self::range_to_ctor(tcx, ty, subrange_lo..=subrange_hi)); + remaining_ranges.push( + Self::range_to_ctor(tcx, ty, subrange_lo..=subrange_hi, self.span), + ); } else { if lo > subrange_lo { // The pattern intersects an upper section of the // subrange, so a lower section will remain. - remaining_ranges.push(Self::range_to_ctor(tcx, ty, subrange_lo..=(lo - 1))); + remaining_ranges.push( + Self::range_to_ctor(tcx, ty, subrange_lo..=(lo - 1), self.span), + ); } if hi < subrange_hi { // The pattern intersects a lower section of the // subrange, so an upper section will remain. - remaining_ranges.push(Self::range_to_ctor(tcx, ty, (hi + 1)..=subrange_hi)); + remaining_ranges.push( + Self::range_to_ctor(tcx, ty, (hi + 1)..=subrange_hi, self.span), + ); } } } @@ -1017,11 +1069,29 @@ impl<'tcx> IntRange<'tcx> { let (lo, hi) = (*self.range.start(), *self.range.end()); let (other_lo, other_hi) = (*other.range.start(), *other.range.end()); if lo <= other_hi && other_lo <= hi { - Some(IntRange { range: max(lo, other_lo)..=min(hi, other_hi), ty }) + let span = other.span; + Some(IntRange { range: max(lo, other_lo)..=min(hi, other_hi), ty, span }) } else { None } } + + fn suspicious_intersection(&self, other: &Self) -> bool { + // `false` in the following cases: + // 1 ---- // 1 ---------- // 1 ---- // 1 ---- + // 2 ---------- // 2 ---- // 2 ---- // 2 ---- + // + // The following are currently `false`, but could be `true` in the future (#64007): + // 1 --------- // 1 --------- + // 2 ---------- // 2 ---------- + // + // `true` in the following cases: + // 1 ------- // 1 ------- + // 2 -------- // 2 ------- + let (lo, hi) = (*self.range.start(), *self.range.end()); + let (other_lo, other_hi) = (*other.range.start(), *other.range.end()); + (lo == other_hi || hi == other_lo) + } } // A request for missing constructor data in terms of either: @@ -1127,6 +1197,7 @@ pub fn is_useful<'p, 'a, 'tcx>( matrix: &Matrix<'p, 'tcx>, v: &[&Pat<'tcx>], witness: WitnessPreference, + hir_id: HirId, ) -> Usefulness<'tcx> { let &Matrix(ref rows) = matrix; debug!("is_useful({:#?}, {:#?})", matrix, v); @@ -1149,6 +1220,10 @@ pub fn is_useful<'p, 'a, 'tcx>( assert!(rows.iter().all(|r| r.len() == v.len())); + let (ty, span) = rows.iter() + .map(|r| (r[0].ty, r[0].span)) + .find(|(ty, _)| !ty.references_error()) + .unwrap_or((v[0].ty, v[0].span)); let pcx = PatCtxt { // TyErr is used to represent the type of wildcard patterns matching // against inaccessible (private) fields of structs, so that we won't @@ -1169,8 +1244,9 @@ pub fn is_useful<'p, 'a, 'tcx>( // FIXME: this might lead to "unstable" behavior with macro hygiene // introducing uninhabited patterns for inaccessible fields. We // need to figure out how to model that. - ty: rows.iter().map(|r| r[0].ty).find(|ty| !ty.references_error()).unwrap_or(v[0].ty), - max_slice_length: max_slice_length(cx, rows.iter().map(|r| r[0]).chain(Some(v[0]))) + ty, + max_slice_length: max_slice_length(cx, rows.iter().map(|r| r[0]).chain(Some(v[0]))), + span, }; debug!("is_useful_expand_first_col: pcx={:#?}, expanding {:#?}", pcx, v[0]); @@ -1184,9 +1260,9 @@ pub fn is_useful<'p, 'a, 'tcx>( Useful } else { split_grouped_constructors( - cx.tcx, cx.param_env, constructors, matrix, pcx.ty, + cx.tcx, cx.param_env, constructors, matrix, pcx.ty, pcx.span, Some(hir_id), ).into_iter().map(|c| - is_useful_specialized(cx, matrix, v, c, pcx.ty, witness) + is_useful_specialized(cx, matrix, v, c, pcx.ty, witness, hir_id) ).find(|result| result.is_useful()).unwrap_or(NotUseful) } } else { @@ -1239,8 +1315,11 @@ pub fn is_useful<'p, 'a, 'tcx>( (pcx.ty.is_ptr_sized_integral() && !cx.tcx.features().precise_pointer_size_matching); if cheap_missing_ctors == MissingCtors::Empty && !is_non_exhaustive { - split_grouped_constructors(cx.tcx, cx.param_env, all_ctors, matrix, pcx.ty) - .into_iter().map(|c| is_useful_specialized(cx, matrix, v, c, pcx.ty, witness)) + split_grouped_constructors( + cx.tcx, cx.param_env, all_ctors, matrix, pcx.ty, DUMMY_SP, None, + ) + .into_iter() + .map(|c| is_useful_specialized(cx, matrix, v, c, pcx.ty, witness, hir_id)) .find(|result| result.is_useful()) .unwrap_or(NotUseful) } else { @@ -1251,7 +1330,7 @@ pub fn is_useful<'p, 'a, 'tcx>( None } }).collect(); - match is_useful(cx, &matrix, &v[1..], witness) { + match is_useful(cx, &matrix, &v[1..], witness, hir_id) { UsefulWithWitness(pats) => { let cx = &*cx; // In this case, there's at least one "free" @@ -1344,6 +1423,7 @@ fn is_useful_specialized<'p, 'a, 'tcx>( ctor: Constructor<'tcx>, lty: Ty<'tcx>, witness: WitnessPreference, + hir_id: HirId, ) -> Usefulness<'tcx> { debug!("is_useful_specialized({:#?}, {:#?}, {:?})", v, ctor, lty); let sub_pat_tys = constructor_sub_pattern_tys(cx, &ctor, lty); @@ -1361,7 +1441,7 @@ fn is_useful_specialized<'p, 'a, 'tcx>( .collect() ); match specialize(cx, v, &ctor, &wild_patterns) { - Some(v) => match is_useful(cx, &matrix, &v, witness) { + Some(v) => match is_useful(cx, &matrix, &v, witness, hir_id) { UsefulWithWitness(witnesses) => UsefulWithWitness( witnesses.into_iter() .map(|witness| witness.apply_constructor(cx, &ctor, lty)) @@ -1381,11 +1461,11 @@ fn is_useful_specialized<'p, 'a, 'tcx>( /// `[a, b, ..tail]` can match a slice of length 2, 3, 4 and so on. /// /// Returns `None` in case of a catch-all, which can't be specialized. -fn pat_constructors<'tcx>(cx: &mut MatchCheckCtxt<'_, 'tcx>, - pat: &Pat<'tcx>, - pcx: PatCtxt<'tcx>) - -> Option>> -{ +fn pat_constructors<'tcx>( + cx: &mut MatchCheckCtxt<'_, 'tcx>, + pat: &Pat<'tcx>, + pcx: PatCtxt<'tcx>, +) -> Option>> { match *pat.kind { PatKind::AscribeUserType { ref subpattern, .. } => pat_constructors(cx, subpattern, pcx), @@ -1394,13 +1474,14 @@ fn pat_constructors<'tcx>(cx: &mut MatchCheckCtxt<'_, 'tcx>, PatKind::Variant { adt_def, variant_index, .. } => { Some(vec![Variant(adt_def.variants[variant_index].def_id)]) } - PatKind::Constant { value } => Some(vec![ConstantValue(value)]), + PatKind::Constant { value } => Some(vec![ConstantValue(value, pat.span)]), PatKind::Range(PatRange { lo, hi, end }) => Some(vec![ConstantRange( lo.eval_bits(cx.tcx, cx.param_env, lo.ty), hi.eval_bits(cx.tcx, cx.param_env, hi.ty), lo.ty, end, + pat.span, )]), PatKind::Array { .. } => match pcx.ty.kind { ty::Array(_, length) => Some(vec![ @@ -1433,7 +1514,7 @@ fn constructor_arity(cx: &MatchCheckCtxt<'a, 'tcx>, ctor: &Constructor<'tcx>, ty ty::Tuple(ref fs) => fs.len() as u64, ty::Slice(..) | ty::Array(..) => match *ctor { Slice(length) => length, - ConstantValue(_) => 0, + ConstantValue(..) => 0, _ => bug!("bad slice pattern {:?} {:?}", ctor, ty) } ty::Ref(..) => 1, @@ -1458,7 +1539,7 @@ fn constructor_sub_pattern_tys<'a, 'tcx>( ty::Tuple(ref fs) => fs.into_iter().map(|t| t.expect_ty()).collect(), ty::Slice(ty) | ty::Array(ty, _) => match *ctor { Slice(length) => (0..length).map(|_| ty).collect(), - ConstantValue(_) => vec![], + ConstantValue(..) => vec![], _ => bug!("bad slice pattern {:?} {:?}", ctor, ty) } ty::Ref(_, rty, _) => vec![rty], @@ -1556,8 +1637,8 @@ fn slice_pat_covered_by_const<'tcx>( // constructor is a range or constant with an integer type. fn should_treat_range_exhaustively(tcx: TyCtxt<'tcx>, ctor: &Constructor<'tcx>) -> bool { let ty = match ctor { - ConstantValue(value) => value.ty, - ConstantRange(_, _, ty, _) => ty, + ConstantValue(value, _) => value.ty, + ConstantRange(_, _, ty, _, _) => ty, _ => return false, }; if let ty::Char | ty::Int(_) | ty::Uint(_) = ty.kind { @@ -1599,12 +1680,17 @@ fn should_treat_range_exhaustively(tcx: TyCtxt<'tcx>, ctor: &Constructor<'tcx>) /// boundaries for each interval range, sort them, then create constructors for each new interval /// between every pair of boundary points. (This essentially sums up to performing the intuitive /// merging operation depicted above.) +/// +/// `hir_id` is `None` when we're evaluating the wildcard pattern, do not lint for overlapping in +/// ranges that case. fn split_grouped_constructors<'p, 'tcx>( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, ctors: Vec>, &Matrix(ref m): &Matrix<'p, 'tcx>, ty: Ty<'tcx>, + span: Span, + hir_id: Option, ) -> Vec> { let mut split_ctors = Vec::with_capacity(ctors.len()); @@ -1621,7 +1707,7 @@ fn split_grouped_constructors<'p, 'tcx>( /// Represents a border between 2 integers. Because the intervals spanning borders /// must be able to cover every integer, we need to be able to represent /// 2^128 + 1 such borders. - #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)] enum Border { JustBefore(u128), AfterMax, @@ -1638,16 +1724,38 @@ fn split_grouped_constructors<'p, 'tcx>( vec![from, to].into_iter() } + // Collect the span and range of all the intersecting ranges to lint on likely + // incorrect range patterns. (#63987) + let mut overlaps = vec![]; // `borders` is the set of borders between equivalence classes: each equivalence // class lies between 2 borders. let row_borders = m.iter() - .flat_map(|row| IntRange::from_pat(tcx, param_env, row[0])) - .flat_map(|range| ctor_range.intersection(&range)) + .flat_map(|row| { + IntRange::from_pat(tcx, param_env, row[0]).map(|r| (r, row.len())) + }) + .flat_map(|(range, row_len)| { + let intersection = ctor_range.intersection(&range); + let should_lint = ctor_range.suspicious_intersection(&range); + if let (Some(range), 1, true) = (&intersection, row_len, should_lint) { + // FIXME: for now, only check for overlapping ranges on simple range + // patterns. Otherwise with the current logic the following is detected + // as overlapping: + // match (10u8, true) { + // (0 ..= 125, false) => {} + // (126 ..= 255, false) => {} + // (0 ..= 255, true) => {} + // } + overlaps.push(range.clone()); + } + intersection + }) .flat_map(|range| range_borders(range)); let ctor_borders = range_borders(ctor_range.clone()); let mut borders: Vec<_> = row_borders.chain(ctor_borders).collect(); borders.sort_unstable(); + lint_overlapping_patterns(tcx, hir_id, ctor_range, ty, overlaps); + // We're going to iterate through every pair of borders, making sure that each // represents an interval of nonnegative length, and convert each such interval // into a constructor. @@ -1655,18 +1763,18 @@ fn split_grouped_constructors<'p, 'tcx>( match (window[0], window[1]) { (Border::JustBefore(n), Border::JustBefore(m)) => { if n < m { - Some(IntRange { range: n..=(m - 1), ty }) + Some(IntRange { range: n..=(m - 1), ty, span }) } else { None } } (Border::JustBefore(n), Border::AfterMax) => { - Some(IntRange { range: n..=u128::MAX, ty }) + Some(IntRange { range: n..=u128::MAX, ty, span }) } (Border::AfterMax, _) => None, } }) { - split_ctors.push(IntRange::range_to_ctor(tcx, ty, range)); + split_ctors.push(IntRange::range_to_ctor(tcx, ty, range, span)); } } // Any other constructor can be used unchanged. @@ -1677,6 +1785,32 @@ fn split_grouped_constructors<'p, 'tcx>( split_ctors } +fn lint_overlapping_patterns( + tcx: TyCtxt<'tcx>, + hir_id: Option, + ctor_range: IntRange<'tcx>, + ty: Ty<'tcx>, + overlaps: Vec>, +) { + if let (true, Some(hir_id)) = (!overlaps.is_empty(), hir_id) { + let mut err = tcx.struct_span_lint_hir( + lint::builtin::OVERLAPPING_PATTERNS, + hir_id, + ctor_range.span, + "multiple patterns covering the same range", + ); + err.span_label(ctor_range.span, "overlapping patterns"); + for int_range in overlaps { + // Use the real type for user display of the ranges: + err.span_label(int_range.span, &format!( + "this range overlaps on `{}`", + IntRange::range_to_ctor(tcx, ty, int_range.range, DUMMY_SP).display(tcx), + )); + } + err.emit(); + } +} + fn constructor_covered_by_range<'tcx>( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, @@ -1701,13 +1835,13 @@ fn constructor_covered_by_range<'tcx>( }; } match *ctor { - ConstantValue(value) => { + ConstantValue(value, _) => { let to = some_or_ok!(cmp_to(value)); let end = (to == Ordering::Less) || (end == RangeEnd::Included && to == Ordering::Equal); Ok(some_or_ok!(cmp_from(value)) && end) }, - ConstantRange(from, to, ty, RangeEnd::Included) => { + ConstantRange(from, to, ty, RangeEnd::Included, _) => { let to = some_or_ok!(cmp_to(ty::Const::from_bits( tcx, to, @@ -1721,7 +1855,7 @@ fn constructor_covered_by_range<'tcx>( ty::ParamEnv::empty().and(ty), ))) && end) }, - ConstantRange(from, to, ty, RangeEnd::Excluded) => { + ConstantRange(from, to, ty, RangeEnd::Excluded, _) => { let to = some_or_ok!(cmp_to(ty::Const::from_bits( tcx, to, @@ -1915,7 +2049,7 @@ fn specialize<'p, 'a: 'p, 'tcx>( None } } - ConstantValue(cv) => { + ConstantValue(cv, _) => { match slice_pat_covered_by_const( cx.tcx, pat.span, cv, prefix, slice, suffix, cx.param_env, ) { diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index 9bed4fb66ea9d..7bc4bf291ee48 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -10,6 +10,7 @@ use rustc::ty::subst::{InternalSubsts, SubstsRef}; use rustc::lint; use rustc_errors::{Applicability, DiagnosticBuilder}; +use rustc::hir::HirId; use rustc::hir::def::*; use rustc::hir::def_id::DefId; use rustc::hir::intravisit::{self, Visitor, NestedVisitorMap}; @@ -239,7 +240,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { .map(|pat| smallvec![pat.0]) .collect(); let scrut_ty = self.tables.node_type(scrut.hir_id); - check_exhaustive(cx, scrut_ty, scrut.span, &matrix); + check_exhaustive(cx, scrut_ty, scrut.span, &matrix, scrut.hir_id); }) } @@ -256,7 +257,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { expand_pattern(cx, pattern) ]].into_iter().collect(); - let witnesses = match check_not_useful(cx, pattern_ty, &pats) { + let witnesses = match check_not_useful(cx, pattern_ty, &pats, pat.hir_id) { Ok(_) => return, Err(err) => err, }; @@ -389,7 +390,7 @@ fn check_arms<'tcx>( for &(pat, hir_pat) in pats { let v = smallvec![pat]; - match is_useful(cx, &seen, &v, LeaveOutWitness) { + match is_useful(cx, &seen, &v, LeaveOutWitness, hir_pat.hir_id) { NotUseful => { match source { hir::MatchSource::IfDesugar { .. } | @@ -465,9 +466,10 @@ fn check_not_useful( cx: &mut MatchCheckCtxt<'_, 'tcx>, ty: Ty<'tcx>, matrix: &Matrix<'_, 'tcx>, + hir_id: HirId, ) -> Result<(), Vec>> { let wild_pattern = super::Pat { ty, span: DUMMY_SP, kind: box PatKind::Wild }; - match is_useful(cx, matrix, &[&wild_pattern], ConstructWitness) { + match is_useful(cx, matrix, &[&wild_pattern], ConstructWitness, hir_id) { NotUseful => Ok(()), // This is good, wildcard pattern isn't reachable. UsefulWithWitness(pats) => Err(if pats.is_empty() { vec![wild_pattern] @@ -483,8 +485,9 @@ fn check_exhaustive<'tcx>( scrut_ty: Ty<'tcx>, sp: Span, matrix: &Matrix<'_, 'tcx>, + hir_id: HirId, ) { - let witnesses = match check_not_useful(cx, scrut_ty, matrix) { + let witnesses = match check_not_useful(cx, scrut_ty, matrix, hir_id) { Ok(_) => return, Err(err) => err, }; diff --git a/src/librustc_mir/hair/pattern/mod.rs b/src/librustc_mir/hair/pattern/mod.rs index 58d741b9295a3..7e17162dfb3ef 100644 --- a/src/librustc_mir/hair/pattern/mod.rs +++ b/src/librustc_mir/hair/pattern/mod.rs @@ -312,10 +312,7 @@ impl<'tcx> fmt::Display for Pat<'tcx> { } PatKind::Range(PatRange { lo, hi, end }) => { write!(f, "{}", lo)?; - match end { - RangeEnd::Included => write!(f, "..=")?, - RangeEnd::Excluded => write!(f, "..")?, - } + write!(f, "{}", end)?; write!(f, "{}", hi) } PatKind::Slice { ref prefix, ref slice, ref suffix } | diff --git a/src/test/ui/check_match/issue-43253.rs b/src/test/ui/check_match/issue-43253.rs index a4d6e9b777f02..5c6834459f0d4 100644 --- a/src/test/ui/check_match/issue-43253.rs +++ b/src/test/ui/check_match/issue-43253.rs @@ -1,7 +1,7 @@ -// build-pass (FIXME(62277): could be check-pass?) - +// check-pass #![feature(exclusive_range_pattern)] #![warn(unreachable_patterns)] +#![warn(overlapping_patterns)] fn main() { // These cases should generate no warning. @@ -13,7 +13,7 @@ fn main() { match 10 { 1..10 => {}, - 9..=10 => {}, + 9..=10 => {}, //~ WARNING multiple patterns covering the same range _ => {}, } @@ -23,22 +23,25 @@ fn main() { _ => {}, } - // These cases should generate an "unreachable pattern" warning. + // These cases should generate "unreachable pattern" warnings. match 10 { 1..10 => {}, - 9 => {}, + 9 => {}, //~ WARNING unreachable pattern _ => {}, } match 10 { 1..10 => {}, - 8..=9 => {}, + 8..=9 => {}, //~ WARNING multiple patterns covering the same range _ => {}, } match 10 { - 1..10 => {}, - 9..=9 => {}, + 5..7 => {}, + 6 => {}, //~ WARNING unreachable pattern + 1..10 => {}, //~ WARNING multiple patterns covering the same range + 9..=9 => {}, //~ WARNING unreachable pattern + 6 => {}, //~ WARNING unreachable pattern _ => {}, } } diff --git a/src/test/ui/check_match/issue-43253.stderr b/src/test/ui/check_match/issue-43253.stderr index d961f623e1fa7..cb4a0486eef9a 100644 --- a/src/test/ui/check_match/issue-43253.stderr +++ b/src/test/ui/check_match/issue-43253.stderr @@ -1,3 +1,17 @@ +warning: multiple patterns covering the same range + --> $DIR/issue-43253.rs:16:9 + | +LL | 1..10 => {}, + | ----- this range overlaps on `9i32` +LL | 9..=10 => {}, + | ^^^^^^ overlapping patterns + | +note: lint level defined here + --> $DIR/issue-43253.rs:4:9 + | +LL | #![warn(overlapping_patterns)] + | ^^^^^^^^^^^^^^^^^^^^ + warning: unreachable pattern --> $DIR/issue-43253.rs:29:9 | @@ -5,7 +19,7 @@ LL | 9 => {}, | ^ | note: lint level defined here - --> $DIR/issue-43253.rs:4:9 + --> $DIR/issue-43253.rs:3:9 | LL | #![warn(unreachable_patterns)] | ^^^^^^^^^^^^^^^^^^^^ @@ -19,6 +33,18 @@ LL | 8..=9 => {}, warning: unreachable pattern --> $DIR/issue-43253.rs:41:9 | +LL | 6 => {}, + | ^ + +warning: unreachable pattern + --> $DIR/issue-43253.rs:43:9 + | LL | 9..=9 => {}, | ^^^^^ +warning: unreachable pattern + --> $DIR/issue-43253.rs:44:9 + | +LL | 6 => {}, + | ^ + diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs index 2570bc8a56078..59f749198971b 100644 --- a/src/test/ui/exhaustive_integer_patterns.rs +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -1,7 +1,7 @@ #![feature(precise_pointer_size_matching)] #![feature(exclusive_range_pattern)] - #![deny(unreachable_patterns)] +#![deny(overlapping_patterns)] use std::{char, u8, u16, u32, u64, u128, i8, i16, i32, i64, i128}; @@ -41,7 +41,8 @@ fn main() { match x { //~ ERROR non-exhaustive patterns -7 => {} -5..=120 => {} - -2..=20 => {} //~ ERROR unreachable pattern + -2..=20 => {} + //~^ ERROR unreachable pattern 125 => {} } @@ -135,9 +136,9 @@ fn main() { (125 .. 128, false) => {} } - match 0u8 { // ok + match 0u8 { 0 .. 2 => {} - 1 ..= 2 => {} + 1 ..= 2 => {} //~ ERROR multiple patterns covering the same range _ => {} } diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr index 6c4b7b0cc0352..7a3a36a820c65 100644 --- a/src/test/ui/exhaustive_integer_patterns.stderr +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -5,7 +5,7 @@ LL | 200 => {} | ^^^ | note: lint level defined here - --> $DIR/exhaustive_integer_patterns.rs:4:9 + --> $DIR/exhaustive_integer_patterns.rs:3:9 | LL | #![deny(unreachable_patterns)] | ^^^^^^^^^^^^^^^^^^^^ @@ -41,7 +41,7 @@ LL | match x { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `std::i8::MIN` not covered - --> $DIR/exhaustive_integer_patterns.rs:82:11 + --> $DIR/exhaustive_integer_patterns.rs:83:11 | LL | match 0i8 { | ^^^ pattern `std::i8::MIN` not covered @@ -49,7 +49,7 @@ LL | match 0i8 { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `0i16` not covered - --> $DIR/exhaustive_integer_patterns.rs:90:11 + --> $DIR/exhaustive_integer_patterns.rs:91:11 | LL | match 0i16 { | ^^^^ pattern `0i16` not covered @@ -57,7 +57,7 @@ LL | match 0i16 { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `128u8..=std::u8::MAX` not covered - --> $DIR/exhaustive_integer_patterns.rs:108:11 + --> $DIR/exhaustive_integer_patterns.rs:109:11 | LL | match 0u8 { | ^^^ pattern `128u8..=std::u8::MAX` not covered @@ -65,7 +65,7 @@ LL | match 0u8 { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `(0u8, Some(_))` and `(2u8..=std::u8::MAX, Some(_))` not covered - --> $DIR/exhaustive_integer_patterns.rs:120:11 + --> $DIR/exhaustive_integer_patterns.rs:121:11 | LL | match (0u8, Some(())) { | ^^^^^^^^^^^^^^^ patterns `(0u8, Some(_))` and `(2u8..=std::u8::MAX, Some(_))` not covered @@ -73,15 +73,29 @@ LL | match (0u8, Some(())) { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `(126u8..=127u8, false)` not covered - --> $DIR/exhaustive_integer_patterns.rs:125:11 + --> $DIR/exhaustive_integer_patterns.rs:126:11 | LL | match (0u8, true) { | ^^^^^^^^^^^ pattern `(126u8..=127u8, false)` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms +error: multiple patterns covering the same range + --> $DIR/exhaustive_integer_patterns.rs:141:9 + | +LL | 0 .. 2 => {} + | ------ this range overlaps on `1u8` +LL | 1 ..= 2 => {} + | ^^^^^^^ overlapping patterns + | +note: lint level defined here + --> $DIR/exhaustive_integer_patterns.rs:4:9 + | +LL | #![deny(overlapping_patterns)] + | ^^^^^^^^^^^^^^^^^^^^ + error[E0004]: non-exhaustive patterns: `std::u128::MAX` not covered - --> $DIR/exhaustive_integer_patterns.rs:145:11 + --> $DIR/exhaustive_integer_patterns.rs:146:11 | LL | match 0u128 { | ^^^^^ pattern `std::u128::MAX` not covered @@ -89,7 +103,7 @@ LL | match 0u128 { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `5u128..=std::u128::MAX` not covered - --> $DIR/exhaustive_integer_patterns.rs:149:11 + --> $DIR/exhaustive_integer_patterns.rs:150:11 | LL | match 0u128 { | ^^^^^ pattern `5u128..=std::u128::MAX` not covered @@ -97,13 +111,13 @@ LL | match 0u128 { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `0u128..=3u128` not covered - --> $DIR/exhaustive_integer_patterns.rs:153:11 + --> $DIR/exhaustive_integer_patterns.rs:154:11 | LL | match 0u128 { | ^^^^^ pattern `0u128..=3u128` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms -error: aborting due to 13 previous errors +error: aborting due to 14 previous errors For more information about this error, try `rustc --explain E0004`. diff --git a/src/test/ui/issues/issue-13867.rs b/src/test/ui/issues/issue-13867.rs index e66368f9ba8fa..9510aae775341 100644 --- a/src/test/ui/issues/issue-13867.rs +++ b/src/test/ui/issues/issue-13867.rs @@ -2,7 +2,6 @@ // Test that codegen works correctly when there are multiple refutable // patterns in match expression. - enum Foo { FooUint(usize), FooNullary, diff --git a/src/test/ui/issues/issue-21475.rs b/src/test/ui/issues/issue-21475.rs index 16d003aba7cab..ab0a18869632a 100644 --- a/src/test/ui/issues/issue-21475.rs +++ b/src/test/ui/issues/issue-21475.rs @@ -1,5 +1,5 @@ // run-pass -#![allow(unused_imports)] +#![allow(unused_imports, overlapping_patterns)] // pretty-expanded FIXME #23616 use m::{START, END}; diff --git a/src/test/ui/issues/issue-26251.rs b/src/test/ui/issues/issue-26251.rs index 0434ef9e5a958..edb06fea8ad53 100644 --- a/src/test/ui/issues/issue-26251.rs +++ b/src/test/ui/issues/issue-26251.rs @@ -1,4 +1,6 @@ // run-pass +#![allow(overlapping_patterns)] + fn main() { let x = 'a'; diff --git a/src/test/ui/match/match-range-fail-dominate.rs b/src/test/ui/match/match-range-fail-dominate.rs index a0cc773d20edd..7de7b7e79be44 100644 --- a/src/test/ui/match/match-range-fail-dominate.rs +++ b/src/test/ui/match/match-range-fail-dominate.rs @@ -1,39 +1,45 @@ -//error-pattern: unreachable -//error-pattern: unreachable -//error-pattern: unreachable -//error-pattern: unreachable -//error-pattern: unreachable - -#![deny(unreachable_patterns)] +#![deny(unreachable_patterns, overlapping_patterns)] fn main() { match 5 { 1 ..= 10 => { } 5 ..= 6 => { } + //~^ ERROR unreachable pattern _ => {} }; match 5 { 3 ..= 6 => { } 4 ..= 6 => { } + //~^ ERROR unreachable pattern _ => {} }; match 5 { 4 ..= 6 => { } 4 ..= 6 => { } + //~^ ERROR unreachable pattern _ => {} }; match 'c' { 'A' ..= 'z' => {} 'a' ..= 'z' => {} + //~^ ERROR unreachable pattern _ => {} }; match 1.0f64 { 0.01f64 ..= 6.5f64 => {} - 0.02f64 => {} + //~^ WARNING floating-point types cannot be used in patterns + //~| WARNING floating-point types cannot be used in patterns + //~| WARNING floating-point types cannot be used in patterns + //~| WARNING this was previously accepted by the compiler + //~| WARNING this was previously accepted by the compiler + //~| WARNING this was previously accepted by the compiler + 0.02f64 => {} //~ ERROR unreachable pattern + //~^ WARNING floating-point types cannot be used in patterns + //~| WARNING this was previously accepted by the compiler _ => {} }; } diff --git a/src/test/ui/match/match-range-fail-dominate.stderr b/src/test/ui/match/match-range-fail-dominate.stderr index d0ff4930a4519..c15186d2558f2 100644 --- a/src/test/ui/match/match-range-fail-dominate.stderr +++ b/src/test/ui/match/match-range-fail-dominate.stderr @@ -1,35 +1,35 @@ error: unreachable pattern - --> $DIR/match-range-fail-dominate.rs:12:7 + --> $DIR/match-range-fail-dominate.rs:6:7 | LL | 5 ..= 6 => { } | ^^^^^^^ | note: lint level defined here - --> $DIR/match-range-fail-dominate.rs:7:9 + --> $DIR/match-range-fail-dominate.rs:1:9 | -LL | #![deny(unreachable_patterns)] +LL | #![deny(unreachable_patterns, overlapping_patterns)] | ^^^^^^^^^^^^^^^^^^^^ error: unreachable pattern - --> $DIR/match-range-fail-dominate.rs:18:7 + --> $DIR/match-range-fail-dominate.rs:13:7 | LL | 4 ..= 6 => { } | ^^^^^^^ error: unreachable pattern - --> $DIR/match-range-fail-dominate.rs:24:7 + --> $DIR/match-range-fail-dominate.rs:20:7 | LL | 4 ..= 6 => { } | ^^^^^^^ error: unreachable pattern - --> $DIR/match-range-fail-dominate.rs:30:7 + --> $DIR/match-range-fail-dominate.rs:27:7 | LL | 'a' ..= 'z' => {} | ^^^^^^^^^^^ warning: floating-point types cannot be used in patterns - --> $DIR/match-range-fail-dominate.rs:35:7 + --> $DIR/match-range-fail-dominate.rs:33:7 | LL | 0.01f64 ..= 6.5f64 => {} | ^^^^^^^ @@ -39,7 +39,7 @@ LL | 0.01f64 ..= 6.5f64 => {} = note: for more information, see issue #41620 warning: floating-point types cannot be used in patterns - --> $DIR/match-range-fail-dominate.rs:35:19 + --> $DIR/match-range-fail-dominate.rs:33:19 | LL | 0.01f64 ..= 6.5f64 => {} | ^^^^^^ @@ -48,7 +48,7 @@ LL | 0.01f64 ..= 6.5f64 => {} = note: for more information, see issue #41620 warning: floating-point types cannot be used in patterns - --> $DIR/match-range-fail-dominate.rs:36:7 + --> $DIR/match-range-fail-dominate.rs:40:7 | LL | 0.02f64 => {} | ^^^^^^^ @@ -57,13 +57,13 @@ LL | 0.02f64 => {} = note: for more information, see issue #41620 error: unreachable pattern - --> $DIR/match-range-fail-dominate.rs:36:7 + --> $DIR/match-range-fail-dominate.rs:40:7 | LL | 0.02f64 => {} | ^^^^^^^ warning: floating-point types cannot be used in patterns - --> $DIR/match-range-fail-dominate.rs:35:7 + --> $DIR/match-range-fail-dominate.rs:33:7 | LL | 0.01f64 ..= 6.5f64 => {} | ^^^^^^^ diff --git a/src/test/ui/precise_pointer_size_matching.rs b/src/test/ui/precise_pointer_size_matching.rs index 759b63b188b35..54aeb8616d959 100644 --- a/src/test/ui/precise_pointer_size_matching.rs +++ b/src/test/ui/precise_pointer_size_matching.rs @@ -8,7 +8,7 @@ #![feature(precise_pointer_size_matching)] #![feature(exclusive_range_pattern)] -#![deny(unreachable_patterns)] +#![deny(unreachable_patterns, overlapping_patterns)] use std::{usize, isize}; diff --git a/src/test/ui/rfcs/rfc-2005-default-binding-mode/range.rs b/src/test/ui/rfcs/rfc-2005-default-binding-mode/range.rs index 580e67513b396..f8abd1b96d80e 100644 --- a/src/test/ui/rfcs/rfc-2005-default-binding-mode/range.rs +++ b/src/test/ui/rfcs/rfc-2005-default-binding-mode/range.rs @@ -3,7 +3,7 @@ pub fn main() { let i = 5; match &&&&i { 1 ..= 3 => panic!(), - 3 ..= 8 => {}, + 4 ..= 8 => {}, _ => panic!(), } }