From e3357d99843fc803affb4f67ae0ac407afcb0872 Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 19 May 2018 15:26:07 +0100 Subject: [PATCH 01/47] Implement interval checking --- src/librustc_mir/hair/pattern/_match.rs | 174 ++++++++++++++++++++++-- 1 file changed, 164 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 5d45955771185..ea2597c4f854a 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -273,7 +273,7 @@ impl<'tcx> Constructor<'tcx> { } } -#[derive(Clone)] +#[derive(Clone, Debug)] pub enum Usefulness<'tcx> { Useful, UsefulWithWitness(Vec>), @@ -425,10 +425,10 @@ impl<'tcx> Witness<'tcx> { /// Option we do not include Some(_) in the returned list of constructors. fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, pcx: PatternContext<'tcx>) - -> Vec> + -> (Vec>, bool) { debug!("all_constructors({:?})", pcx.ty); - match pcx.ty.sty { + (match pcx.ty.sty { ty::TyBool => { [true, false].iter().map(|&b| { ConstantValue(ty::Const::from_bool(cx.tcx, b)) @@ -457,6 +457,13 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, .map(|v| Variant(v.did)) .collect() } + ty::TyUint(ast::UintTy::Usize) => { + return (vec![ + ConstantRange(ty::Const::from_usize(cx.tcx, 0), + ty::Const::from_usize(cx.tcx, 100), + RangeEnd::Excluded), + ], true) + } _ => { if cx.is_uninhabited(pcx.ty) { vec![] @@ -464,7 +471,7 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, vec![Single] } } - } + }, false) } fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( @@ -656,11 +663,148 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, pat_constructors(cx, row[0], pcx).unwrap_or(vec![]) }).collect(); debug!("used_ctors = {:#?}", used_ctors); - let all_ctors = all_constructors(cx, pcx); + let (all_ctors, _ranged) = all_constructors(cx, pcx); debug!("all_ctors = {:#?}", all_ctors); - let missing_ctors: Vec = all_ctors.iter().filter(|c| { - !used_ctors.contains(*c) - }).cloned().collect(); + + fn to_inc_range_pair<'tcx>(tcx: TyCtxt<'_, '_, '_>, ctor: &Constructor<'tcx>) -> Option<(u64, u64)> { + match ctor { + Single | Variant(_) | Slice(_) => { + None + } + ConstantValue(const_) => { + if let Some(val) = const_.assert_usize(tcx) { + return Some((val, val)); + } + None + } + ConstantRange(lo, hi, end) => { + if let Some(lo) = lo.assert_usize(tcx) { + if let Some(hi) = hi.assert_usize(tcx) { + if lo > hi || lo == hi && end == &RangeEnd::Excluded { + return None; + } else if end == &RangeEnd::Included { + return Some((lo, hi)); + } else { + return Some((lo, hi - 1)); + } + } + } + None + } + } + } + + fn intersect<'a, 'tcx>( + _deb: bool, + cx: &mut MatchCheckCtxt<'a, 'tcx>, + ranges: Vec>, + ctor: &Constructor<'tcx>) + -> (Vec>, bool) { + if let Some((lo1, hi1)) = to_inc_range_pair(cx.tcx, ctor) { + let mut ctor_was_useful = false; + // values only consists of ranges + let mut new_ranges = vec![]; + let mut ranges: Vec<_> = + ranges.into_iter().filter_map(|r| to_inc_range_pair(cx.tcx, &r)).collect(); + while let Some((lo2, hi2)) = ranges.pop() { + eprintln!("{:?} {:?}", (lo2, hi2), (lo1, hi1)); + if lo1 <= lo2 && hi1 >= hi2 { + if _deb { eprintln!("case 1"); } + ctor_was_useful = true; + continue; + } + if lo1 > hi2 || hi1 < lo2 { + if _deb { eprintln!("case 2"); } + new_ranges.push((lo2, hi2)); + continue; + } + if lo1 <= lo2 { + if _deb { eprintln!("case 3"); } + ctor_was_useful = true; + if (hi1 + 1, hi2) == (lo2, hi2) { + new_ranges.push((hi1 + 1, hi2)); + } else { + ranges.push((hi1 + 1, hi2)); + } + continue; + } + if hi1 >= hi2 { + if _deb { eprintln!("case 4"); } + ctor_was_useful = true; + if (lo2, lo1 - 1) == (lo2, hi2) { + new_ranges.push((lo2, lo1 - 1)); + } else { + ranges.push((lo2, lo1 - 1)); + } + continue; + } + ctor_was_useful = true; + ranges.push((lo2, lo1)); + ranges.push((hi1, hi2)); + if _deb { eprintln!("case 5"); } + } + // transform ranges to proper format + (new_ranges.into_iter().map(|(lo, hi)| { + ConstantRange(ty::Const::from_usize(cx.tcx, lo), + ty::Const::from_usize(cx.tcx, hi), + RangeEnd::Included) + }).collect(), ctor_was_useful) + } else { + (ranges, false) + } + } + + // `used_ctors` are all the constructors that appear in patterns (must check if guards) + // `all_ctors` are all the necessary constructors + let mut missing_ctors = vec![]; + let mut all_actual_ctors = vec![]; + 'req: for req_ctor in all_ctors.clone() { + if _deb { + eprintln!("req_ctor before {:?}", req_ctor); + } + let mut cur = vec![req_ctor.clone()]; + for used_ctor in &used_ctors { + if _deb { + eprintln!("cut {:?}", used_ctor); + } + if cur.iter().all(|ctor| { + match ctor { + ConstantRange(..) => true, + _ => false, + } + }) { + let (cur2, ctor_was_useful) = intersect(_deb, cx, cur, used_ctor); + cur = cur2; + if ctor_was_useful { + all_actual_ctors.push(used_ctor.clone()); + } + if cur.is_empty() { + continue 'req; + } + } else { + if used_ctor == &req_ctor { + continue 'req; + } + } + } + if _deb { + eprintln!("req_ctor after {:?}", cur); + } + missing_ctors.extend(cur); + } + + // let missing_ctors: Vec = all_ctors.iter().filter(|c| { + // !used_ctors.contains(*c) + // }).cloned().collect(); + + if _deb { + eprintln!("used_ctors {:?}", used_ctors); + eprintln!("missing_ctors {:?}", missing_ctors); + } + + // if !all_actual_ctors.is_empty() { + // all_ctors = all_actual_ctors; + // } // `missing_ctors` is the set of constructors from the same type as the // first column of `matrix` that are matched only by wildcard patterns @@ -693,10 +837,16 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let is_non_exhaustive = is_privately_empty || is_declared_nonexhaustive; if missing_ctors.is_empty() && !is_non_exhaustive { - all_ctors.into_iter().map(|c| { + if _ranged && _deb { + return NotUseful; + } + let z = all_ctors.into_iter().map(|c| { is_useful_specialized(cx, matrix, v, c.clone(), pcx.ty, witness) - }).find(|result| result.is_useful()).unwrap_or(NotUseful) + }).find(|result| result.is_useful()).unwrap_or(NotUseful); + if _deb { eprintln!("ABC 1 {:?}", z); } + z } else { + if _deb { eprintln!("ABC 2"); } let matrix = rows.iter().filter_map(|r| { if r[0].is_wildcard() { Some(r[1..].to_vec()) @@ -706,6 +856,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, }).collect(); match is_useful(cx, &matrix, &v[1..], witness) { UsefulWithWitness(pats) => { + if _deb { eprintln!("ABC 3"); } let cx = &*cx; // In this case, there's at least one "free" // constructor that is only matched against by @@ -752,6 +903,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // satisfied with `(_, _, true)`. In this case, // `used_ctors` is empty. let new_witnesses = if is_non_exhaustive || used_ctors.is_empty() { + if _deb { eprintln!("ABC 4"); } // All constructors are unused. Add wild patterns // rather than each individual constructor pats.into_iter().map(|mut witness| { @@ -763,6 +915,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, witness }).collect() } else { + if _deb { eprintln!("ABC 5"); } pats.into_iter().flat_map(|witness| { missing_ctors.iter().map(move |ctor| { witness.clone().push_wild_constructor(cx, ctor, pcx.ty) @@ -1062,6 +1215,7 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( PatternKind::Leaf { ref subpatterns } => { Some(patterns_for_variant(subpatterns, wild_patterns)) } + PatternKind::Deref { ref subpattern } => { Some(vec![subpattern]) } From b3d2baff9485f3a6e54ab606c188c80fab76f13d Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 19 May 2018 18:46:05 +0100 Subject: [PATCH 02/47] Give correct suggestions --- src/librustc_mir/hair/pattern/_match.rs | 45 +++++++++++++++++++++---- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index ea2597c4f854a..4460587c614df 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -707,7 +707,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let mut ranges: Vec<_> = ranges.into_iter().filter_map(|r| to_inc_range_pair(cx.tcx, &r)).collect(); while let Some((lo2, hi2)) = ranges.pop() { - eprintln!("{:?} {:?}", (lo2, hi2), (lo1, hi1)); + // eprintln!("{:?} {:?}", (lo2, hi2), (lo1, hi1)); if lo1 <= lo2 && hi1 >= hi2 { if _deb { eprintln!("case 1"); } ctor_was_useful = true; @@ -793,6 +793,17 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, missing_ctors.extend(cur); } + // if _ranged { + // missing_ctors = missing_ctors.into_iter().map(|ctor| { + // match ctor { + // ConstantRange(lo, hi, RangeEnd::Included) if lo == hi => { + // ConstantValue(lo) + // } + // _ => ctor, + // } + // }).collect(); + // } + // let missing_ctors: Vec = all_ctors.iter().filter(|c| { // !used_ctors.contains(*c) // }).cloned().collect(); @@ -916,11 +927,33 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, }).collect() } else { if _deb { eprintln!("ABC 5"); } - pats.into_iter().flat_map(|witness| { - missing_ctors.iter().map(move |ctor| { - witness.clone().push_wild_constructor(cx, ctor, pcx.ty) - }) - }).collect() + if _ranged { + missing_ctors.into_iter().map(|ctor| { + match ctor { + ConstantRange(lo, hi, _) if lo == hi => { + Witness(vec![Pattern { + ty: pcx.ty, + span: DUMMY_SP, + kind: box PatternKind::Constant { value: lo }, + }]) + } + ConstantRange(lo, hi, end) => { + Witness(vec![Pattern { + ty: pcx.ty, + span: DUMMY_SP, + kind: box PatternKind::Range { lo, hi, end }, + }]) + }, + _ => bug!("this shouldn't be happening"), + } + }).collect() + } else { + pats.into_iter().flat_map(|witness| { + missing_ctors.iter().map(move |ctor| { + witness.clone().push_wild_constructor(cx, ctor, pcx.ty) + }) + }).collect() + } }; UsefulWithWitness(new_witnesses) } From 384db4f0cff4e7a01707a23be0fad697467c8b1b Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 19 May 2018 21:57:25 +0100 Subject: [PATCH 03/47] Add support for all integer types --- src/librustc_mir/hair/pattern/_match.rs | 74 +++++++++++++++++++------ 1 file changed, 58 insertions(+), 16 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 4460587c614df..670e0b1fef5e5 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -457,13 +457,52 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, .map(|v| Variant(v.did)) .collect() } - ty::TyUint(ast::UintTy::Usize) => { + ty::TyChar => { + let (min, max) = (0u128, char::MAX as u128); return (vec![ - ConstantRange(ty::Const::from_usize(cx.tcx, 0), - ty::Const::from_usize(cx.tcx, 100), - RangeEnd::Excluded), + ConstantRange(ty::Const::from_bits(cx.tcx, min, cx.tcx.types.char), + ty::Const::from_bits(cx.tcx, max, cx.tcx.types.char), + RangeEnd::Included), ], true) } + ty::TyInt(int_ty) => { + use syntax::ast::IntTy::*; + let (min, max, ty) = match int_ty { + Isize => (isize::MIN as i128, isize::MAX as i128, cx.tcx.types.isize), + I8 => ( i8::MIN as i128, i8::MAX as i128, cx.tcx.types.i8), + I16 => ( i16::MIN as i128, i16::MAX as i128, cx.tcx.types.i16), + I32 => ( i32::MIN as i128, i32::MAX as i128, cx.tcx.types.i32), + I64 => ( i64::MIN as i128, i64::MAX as i128, cx.tcx.types.i64), + I128 => ( i128::MIN as i128, i128::MAX as i128, cx.tcx.types.i128), + }; + return (vec![ + ConstantRange( + ty::Const::from_bits(cx.tcx, unsafe { + transmute::(min) + }, ty), + ty::Const::from_bits(cx.tcx, unsafe { + transmute::(max) + }, ty), + RangeEnd::Included + ), + ], true); + } + ty::TyUint(uint_ty) => { + use syntax::ast::UintTy::*; + let (min, (max, ty)) = (0u128, match uint_ty { + Usize => (usize::MAX as u128, cx.tcx.types.usize), + U8 => ( u8::MAX as u128, cx.tcx.types.u8), + U16 => ( u16::MAX as u128, cx.tcx.types.u16), + U32 => ( u32::MAX as u128, cx.tcx.types.u32), + U64 => ( u64::MAX as u128, cx.tcx.types.u64), + U128 => ( u128::MAX as u128, cx.tcx.types.u128), + }); + return (vec![ + ConstantRange(ty::Const::from_bits(cx.tcx, min, ty), + ty::Const::from_bits(cx.tcx, max, ty), + RangeEnd::Included), + ], true); + } _ => { if cx.is_uninhabited(pcx.ty) { vec![] @@ -666,26 +705,27 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let (all_ctors, _ranged) = all_constructors(cx, pcx); debug!("all_ctors = {:#?}", all_ctors); - fn to_inc_range_pair<'tcx>(tcx: TyCtxt<'_, '_, '_>, ctor: &Constructor<'tcx>) -> Option<(u64, u64)> { + fn to_inc_range_pair<'tcx>(_tcx: TyCtxt<'_, '_, '_>, ctor: &Constructor<'tcx>) -> Option<(u128, u128, Ty<'tcx>)> { match ctor { Single | Variant(_) | Slice(_) => { None } ConstantValue(const_) => { - if let Some(val) = const_.assert_usize(tcx) { - return Some((val, val)); + if let Some(val) = const_.assert_bits(const_.ty) { + return Some((val, val, const_.ty)); } None } ConstantRange(lo, hi, end) => { - if let Some(lo) = lo.assert_usize(tcx) { - if let Some(hi) = hi.assert_usize(tcx) { + let ty = lo.ty; + if let Some(lo) = lo.assert_bits(lo.ty) { + if let Some(hi) = hi.assert_bits(hi.ty) { if lo > hi || lo == hi && end == &RangeEnd::Excluded { return None; } else if end == &RangeEnd::Included { - return Some((lo, hi)); + return Some((lo, hi, ty)); } else { - return Some((lo, hi - 1)); + return Some((lo, hi - 1, ty)); } } } @@ -700,12 +740,14 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ranges: Vec>, ctor: &Constructor<'tcx>) -> (Vec>, bool) { - if let Some((lo1, hi1)) = to_inc_range_pair(cx.tcx, ctor) { + if let Some((lo1, hi1, ty)) = to_inc_range_pair(cx.tcx, ctor) { let mut ctor_was_useful = false; // values only consists of ranges let mut new_ranges = vec![]; let mut ranges: Vec<_> = - ranges.into_iter().filter_map(|r| to_inc_range_pair(cx.tcx, &r)).collect(); + ranges.into_iter().filter_map(|r| { + to_inc_range_pair(cx.tcx, &r).map(|(lo, hi, _)| (lo, hi)) + }).collect(); while let Some((lo2, hi2)) = ranges.pop() { // eprintln!("{:?} {:?}", (lo2, hi2), (lo1, hi1)); if lo1 <= lo2 && hi1 >= hi2 { @@ -745,9 +787,9 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } // transform ranges to proper format (new_ranges.into_iter().map(|(lo, hi)| { - ConstantRange(ty::Const::from_usize(cx.tcx, lo), - ty::Const::from_usize(cx.tcx, hi), - RangeEnd::Included) + ConstantRange(ty::Const::from_bits(cx.tcx, lo, ty), + ty::Const::from_bits(cx.tcx, hi, ty), + RangeEnd::Included) }).collect(), ctor_was_useful) } else { (ranges, false) From ed5a4d5e600529ad2df168da497b9d2995da2a8c Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 19 May 2018 23:19:29 +0100 Subject: [PATCH 04/47] Add feature gate and refactor --- src/librustc_mir/hair/pattern/_match.rs | 343 +++++++++++------------- src/libsyntax/feature_gate.rs | 3 + 2 files changed, 164 insertions(+), 182 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 670e0b1fef5e5..5941e71cfa2ba 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -428,7 +428,9 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, -> (Vec>, bool) { debug!("all_constructors({:?})", pcx.ty); - (match pcx.ty.sty { + let exhaustive_integer_patterns = cx.tcx.features().exhaustive_integer_patterns; + let mut value_constructors = false; + let ctors = match pcx.ty.sty { ty::TyBool => { [true, false].iter().map(|&b| { ConstantValue(ty::Const::from_bool(cx.tcx, b)) @@ -457,15 +459,14 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, .map(|v| Variant(v.did)) .collect() } - ty::TyChar => { + ty::TyChar if exhaustive_integer_patterns => { let (min, max) = (0u128, char::MAX as u128); - return (vec![ - ConstantRange(ty::Const::from_bits(cx.tcx, min, cx.tcx.types.char), - ty::Const::from_bits(cx.tcx, max, cx.tcx.types.char), - RangeEnd::Included), - ], true) + value_constructors = true; + vec![ConstantRange(ty::Const::from_bits(cx.tcx, min, cx.tcx.types.char), + ty::Const::from_bits(cx.tcx, max, cx.tcx.types.char), + RangeEnd::Included)] } - ty::TyInt(int_ty) => { + ty::TyInt(int_ty) if exhaustive_integer_patterns => { use syntax::ast::IntTy::*; let (min, max, ty) = match int_ty { Isize => (isize::MIN as i128, isize::MAX as i128, cx.tcx.types.isize), @@ -475,19 +476,16 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, I64 => ( i64::MIN as i128, i64::MAX as i128, cx.tcx.types.i64), I128 => ( i128::MIN as i128, i128::MAX as i128, cx.tcx.types.i128), }; - return (vec![ - ConstantRange( - ty::Const::from_bits(cx.tcx, unsafe { - transmute::(min) - }, ty), - ty::Const::from_bits(cx.tcx, unsafe { - transmute::(max) - }, ty), - RangeEnd::Included - ), - ], true); + value_constructors = true; + vec![ConstantRange(ty::Const::from_bits(cx.tcx, unsafe { + transmute::(min) + }, ty), + ty::Const::from_bits(cx.tcx, unsafe { + transmute::(max) + }, ty), + RangeEnd::Included)] } - ty::TyUint(uint_ty) => { + ty::TyUint(uint_ty) if exhaustive_integer_patterns => { use syntax::ast::UintTy::*; let (min, (max, ty)) = (0u128, match uint_ty { Usize => (usize::MAX as u128, cx.tcx.types.usize), @@ -497,11 +495,10 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, U64 => ( u64::MAX as u128, cx.tcx.types.u64), U128 => ( u128::MAX as u128, cx.tcx.types.u128), }); - return (vec![ - ConstantRange(ty::Const::from_bits(cx.tcx, min, ty), - ty::Const::from_bits(cx.tcx, max, ty), - RangeEnd::Included), - ], true); + value_constructors = true; + vec![ConstantRange(ty::Const::from_bits(cx.tcx, min, ty), + ty::Const::from_bits(cx.tcx, max, ty), + RangeEnd::Included)] } _ => { if cx.is_uninhabited(pcx.ty) { @@ -510,7 +507,8 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, vec![Single] } } - }, false) + }; + (ctors, value_constructors) } fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( @@ -615,6 +613,94 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( cmp::max(max_fixed_len + 1, max_prefix_len + max_suffix_len) } +/// An inclusive interval, used for precise integer exhaustiveness checking. +struct Interval<'tcx> { + pub lo: u128, + pub hi: u128, + pub ty: Ty<'tcx>, +} + +impl<'tcx> Interval<'tcx> { + fn from_ctor(ctor: &Constructor<'tcx>) -> Option> { + match ctor { + ConstantRange(lo, hi, end) => { + assert_eq!(lo.ty, hi.ty); + let ty = lo.ty; + if let Some(lo) = lo.assert_bits(ty) { + if let Some(hi) = hi.assert_bits(ty) { + // Make sure the interval is well-formed. + return if lo > hi || lo == hi && *end == RangeEnd::Excluded { + None + } else { + let offset = (*end == RangeEnd::Excluded) as u128; + Some(Interval { lo, hi: hi - offset, ty }) + }; + } + } + None + } + ConstantValue(val) => { + let ty = val.ty; + val.assert_bits(ty).map(|val| Interval { lo: val, hi: val, ty }) + } + Single | Variant(_) | Slice(_) => { + None + } + } + } + + fn into_inner(self) -> (u128, u128) { + (self.lo, self.hi) + } +} + +/// Given a pattern in a `match` and a collection of ranges corresponding to the +/// domain of values of a type (say, an integer), return a new collection of ranges +/// corresponding to those ranges minus the ranges covered by the pattern. +fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, + pat_ctor: &Constructor<'tcx>, + ranges: Vec>) + -> Vec> { + if let Some(pat_interval) = Interval::from_ctor(pat_ctor) { + let mut remaining_ranges = vec![]; + let mut ranges: Vec<_> = ranges.into_iter().filter_map(|r| { + Interval::from_ctor(&r).map(|i| i.into_inner()) + }).collect(); + for (subrange_lo, subrange_hi) in ranges { + if pat_interval.lo > subrange_hi || pat_interval.hi < subrange_lo { + // The pattern doesn't intersect with the subrange at all, + // so the subrange remains untouched. + remaining_ranges.push((subrange_lo, subrange_hi)); + } else if pat_interval.lo <= subrange_lo && pat_interval.hi >= subrange_hi { + // The pattern entirely covers the subrange of values, + // so we no longer have to consider this subrange_ + } else if pat_interval.lo <= subrange_lo { + // The pattern intersects a lower section of the subrange, + // so only the upper section will remain. + remaining_ranges.push((pat_interval.hi + 1, subrange_hi)); + } else if pat_interval.hi >= subrange_hi { + // The pattern intersects an upper section of the subrange, + // so only the lower section will remain. + remaining_ranges.push((subrange_lo, pat_interval.lo - 1)); + } else { + // The pattern intersects the middle of the subrange, + // so we create two ranges either side of the intersection.) + remaining_ranges.push((subrange_lo, pat_interval.lo)); + remaining_ranges.push((pat_interval.hi, subrange_hi)); + } + } + // Convert the remaining ranges from pairs to inclusive `ConstantRange`s. + let ty = pat_interval.ty; + remaining_ranges.into_iter().map(|(lo, hi)| { + ConstantRange(ty::Const::from_bits(cx.tcx, lo, ty), + ty::Const::from_bits(cx.tcx, hi, ty), + RangeEnd::Included) + }).collect() + } else { + ranges + } +} + /// 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: @@ -702,163 +788,50 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, pat_constructors(cx, row[0], pcx).unwrap_or(vec![]) }).collect(); debug!("used_ctors = {:#?}", used_ctors); - let (all_ctors, _ranged) = all_constructors(cx, pcx); + // `all_ctors` are all the constructors for the given type, which + // should all be represented (or caught with the wild pattern `_`). + // `value_constructors` is true if we may exhaustively consider all + // the possible values (e.g. integers) of a type as its constructors. + let (all_ctors, value_constructors) = all_constructors(cx, pcx); debug!("all_ctors = {:#?}", all_ctors); - fn to_inc_range_pair<'tcx>(_tcx: TyCtxt<'_, '_, '_>, ctor: &Constructor<'tcx>) -> Option<(u128, u128, Ty<'tcx>)> { - match ctor { - Single | Variant(_) | Slice(_) => { - None - } - ConstantValue(const_) => { - if let Some(val) = const_.assert_bits(const_.ty) { - return Some((val, val, const_.ty)); - } - None - } - ConstantRange(lo, hi, end) => { - let ty = lo.ty; - if let Some(lo) = lo.assert_bits(lo.ty) { - if let Some(hi) = hi.assert_bits(hi.ty) { - if lo > hi || lo == hi && end == &RangeEnd::Excluded { - return None; - } else if end == &RangeEnd::Included { - return Some((lo, hi, ty)); - } else { - return Some((lo, hi - 1, ty)); - } - } - } - None - } - } - } - - fn intersect<'a, 'tcx>( - _deb: bool, - cx: &mut MatchCheckCtxt<'a, 'tcx>, - ranges: Vec>, - ctor: &Constructor<'tcx>) - -> (Vec>, bool) { - if let Some((lo1, hi1, ty)) = to_inc_range_pair(cx.tcx, ctor) { - let mut ctor_was_useful = false; - // values only consists of ranges - let mut new_ranges = vec![]; - let mut ranges: Vec<_> = - ranges.into_iter().filter_map(|r| { - to_inc_range_pair(cx.tcx, &r).map(|(lo, hi, _)| (lo, hi)) - }).collect(); - while let Some((lo2, hi2)) = ranges.pop() { - // eprintln!("{:?} {:?}", (lo2, hi2), (lo1, hi1)); - if lo1 <= lo2 && hi1 >= hi2 { - if _deb { eprintln!("case 1"); } - ctor_was_useful = true; - continue; - } - if lo1 > hi2 || hi1 < lo2 { - if _deb { eprintln!("case 2"); } - new_ranges.push((lo2, hi2)); - continue; - } - if lo1 <= lo2 { - if _deb { eprintln!("case 3"); } - ctor_was_useful = true; - if (hi1 + 1, hi2) == (lo2, hi2) { - new_ranges.push((hi1 + 1, hi2)); - } else { - ranges.push((hi1 + 1, hi2)); - } - continue; - } - if hi1 >= hi2 { - if _deb { eprintln!("case 4"); } - ctor_was_useful = true; - if (lo2, lo1 - 1) == (lo2, hi2) { - new_ranges.push((lo2, lo1 - 1)); - } else { - ranges.push((lo2, lo1 - 1)); - } - continue; - } - ctor_was_useful = true; - ranges.push((lo2, lo1)); - ranges.push((hi1, hi2)); - if _deb { eprintln!("case 5"); } - } - // transform ranges to proper format - (new_ranges.into_iter().map(|(lo, hi)| { - ConstantRange(ty::Const::from_bits(cx.tcx, lo, ty), - ty::Const::from_bits(cx.tcx, hi, ty), - RangeEnd::Included) - }).collect(), ctor_was_useful) - } else { - (ranges, false) - } - } - - // `used_ctors` are all the constructors that appear in patterns (must check if guards) - // `all_ctors` are all the necessary constructors + // `missing_ctors` are those that should have appeared + // as patterns in the `match` expression, but did not. let mut missing_ctors = vec![]; - let mut all_actual_ctors = vec![]; 'req: for req_ctor in all_ctors.clone() { - if _deb { - eprintln!("req_ctor before {:?}", req_ctor); - } - let mut cur = vec![req_ctor.clone()]; + let mut sub_ctors = vec![req_ctor.clone()]; + // The only constructor patterns for which it is valid to + // treat the values as constructors are ranges (see + // `all_constructors` for details). + let consider_value_constructors = value_constructors && match req_ctor { + ConstantRange(..) => true, + _ => false, + }; for used_ctor in &used_ctors { - if _deb { - eprintln!("cut {:?}", used_ctor); - } - if cur.iter().all(|ctor| { - match ctor { - ConstantRange(..) => true, - _ => false, - } - }) { - let (cur2, ctor_was_useful) = intersect(_deb, cx, cur, used_ctor); - cur = cur2; - if ctor_was_useful { - all_actual_ctors.push(used_ctor.clone()); - } - if cur.is_empty() { + if consider_value_constructors { + sub_ctors = ranges_subtract_pattern(cx, used_ctor, sub_ctors); + // If the constructor patterns that have been considered so far + // already cover the entire range of values, then we the + // constructor is not missing, and we can move on to the next one. + if sub_ctors.is_empty() { continue 'req; } } else { - if used_ctor == &req_ctor { + // If the pattern for the required constructor + // appears in the `match`, then it is not missing, + // and we can move on to the next one. + if *used_ctor == req_ctor { continue 'req; } } } - if _deb { - eprintln!("req_ctor after {:?}", cur); - } - missing_ctors.extend(cur); + // If a constructor has not been matched, then it is missing. + // We add `sub_ctors` instead of `req_ctor`, because then we can + // provide more detailed error information about precisely which + // ranges have been omitted. + missing_ctors.extend(sub_ctors); } - // if _ranged { - // missing_ctors = missing_ctors.into_iter().map(|ctor| { - // match ctor { - // ConstantRange(lo, hi, RangeEnd::Included) if lo == hi => { - // ConstantValue(lo) - // } - // _ => ctor, - // } - // }).collect(); - // } - - // let missing_ctors: Vec = all_ctors.iter().filter(|c| { - // !used_ctors.contains(*c) - // }).cloned().collect(); - - if _deb { - eprintln!("used_ctors {:?}", used_ctors); - eprintln!("missing_ctors {:?}", missing_ctors); - } - - // if !all_actual_ctors.is_empty() { - // all_ctors = all_actual_ctors; - // } - // `missing_ctors` is the set of constructors from the same type as the // first column of `matrix` that are matched only by wildcard patterns // from the first column. @@ -890,16 +863,16 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let is_non_exhaustive = is_privately_empty || is_declared_nonexhaustive; if missing_ctors.is_empty() && !is_non_exhaustive { - if _ranged && _deb { - return NotUseful; + if value_constructors { + // If we've successfully matched every value + // of the type, then we're done. + NotUseful + } else { + all_ctors.into_iter().map(|c| { + is_useful_specialized(cx, matrix, v, c.clone(), pcx.ty, witness) + }).find(|result| result.is_useful()).unwrap_or(NotUseful) } - let z = all_ctors.into_iter().map(|c| { - is_useful_specialized(cx, matrix, v, c.clone(), pcx.ty, witness) - }).find(|result| result.is_useful()).unwrap_or(NotUseful); - if _deb { eprintln!("ABC 1 {:?}", z); } - z } else { - if _deb { eprintln!("ABC 2"); } let matrix = rows.iter().filter_map(|r| { if r[0].is_wildcard() { Some(r[1..].to_vec()) @@ -909,7 +882,6 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, }).collect(); match is_useful(cx, &matrix, &v[1..], witness) { UsefulWithWitness(pats) => { - if _deb { eprintln!("ABC 3"); } let cx = &*cx; // In this case, there's at least one "free" // constructor that is only matched against by @@ -956,7 +928,6 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // satisfied with `(_, _, true)`. In this case, // `used_ctors` is empty. let new_witnesses = if is_non_exhaustive || used_ctors.is_empty() { - if _deb { eprintln!("ABC 4"); } // All constructors are unused. Add wild patterns // rather than each individual constructor pats.into_iter().map(|mut witness| { @@ -968,10 +939,15 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, witness }).collect() } else { - if _deb { eprintln!("ABC 5"); } - if _ranged { + if value_constructors { + // If we've been trying to exhaustively match + // over the domain of values for a type, + // then we can provide better diagnostics + // regarding which values were missing. missing_ctors.into_iter().map(|ctor| { match ctor { + // A constant range of length 1 is simply + // a constant value. ConstantRange(lo, hi, _) if lo == hi => { Witness(vec![Pattern { ty: pcx.ty, @@ -979,6 +955,8 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, kind: box PatternKind::Constant { value: lo }, }]) } + // We always report missing intervals + // in terms of inclusive ranges. ConstantRange(lo, hi, end) => { Witness(vec![Pattern { ty: pcx.ty, @@ -986,7 +964,8 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, kind: box PatternKind::Range { lo, hi, end }, }]) }, - _ => bug!("this shouldn't be happening"), + _ => bug!("`ranges_subtract_pattern` should only produce \ + `ConstantRange`s"), } }).collect() } else { diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index f837bead6a085..6109e5ecb61c0 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -487,6 +487,9 @@ declare_features! ( // 'a: { break 'a; } (active, label_break_value, "1.28.0", Some(48594), None), + // Integer match exhaustiveness checking + (active, exhaustive_integer_patterns, "1.28.0", Some(50907), None), + // #[panic_implementation] (active, panic_implementation, "1.28.0", Some(44489), None), From b8702a0b1dd66f422d2666de1d50c344c0f008c0 Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 19 May 2018 23:19:37 +0100 Subject: [PATCH 05/47] Add feature gate test --- .../feature-gate-exhaustive_integer_patterns.rs | 16 ++++++++++++++++ ...ature-gate-exhaustive_integer_patterns.stderr | 9 +++++++++ 2 files changed, 25 insertions(+) create mode 100644 src/test/ui/feature-gate-exhaustive_integer_patterns.rs create mode 100644 src/test/ui/feature-gate-exhaustive_integer_patterns.stderr diff --git a/src/test/ui/feature-gate-exhaustive_integer_patterns.rs b/src/test/ui/feature-gate-exhaustive_integer_patterns.rs new file mode 100644 index 0000000000000..3aa1522945548 --- /dev/null +++ b/src/test/ui/feature-gate-exhaustive_integer_patterns.rs @@ -0,0 +1,16 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + let x: u8 = 0; + match x { //~ ERROR non-exhaustive patterns: `_` not covered + 0 ..= 255 => {} + } +} diff --git a/src/test/ui/feature-gate-exhaustive_integer_patterns.stderr b/src/test/ui/feature-gate-exhaustive_integer_patterns.stderr new file mode 100644 index 0000000000000..63d98f6b5eb64 --- /dev/null +++ b/src/test/ui/feature-gate-exhaustive_integer_patterns.stderr @@ -0,0 +1,9 @@ +error[E0004]: non-exhaustive patterns: `_` not covered + --> $DIR/feature-gate-exhaustive_integer_patterns.rs:13:11 + | +LL | match x { //~ ERROR non-exhaustive patterns: `_` not covered + | ^ pattern `_` not covered + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0004`. From 121fa8d499b5d0db6d4b6c25458bd87ea38decc2 Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 20 May 2018 01:54:22 +0100 Subject: [PATCH 06/47] Fix handling of signed integers --- src/librustc_mir/hair/pattern/_match.rs | 56 +++++++++++++++++++------ 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 5941e71cfa2ba..03b1ea44c753d 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -469,20 +469,16 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ty::TyInt(int_ty) if exhaustive_integer_patterns => { use syntax::ast::IntTy::*; let (min, max, ty) = match int_ty { - Isize => (isize::MIN as i128, isize::MAX as i128, cx.tcx.types.isize), - I8 => ( i8::MIN as i128, i8::MAX as i128, cx.tcx.types.i8), - I16 => ( i16::MIN as i128, i16::MAX as i128, cx.tcx.types.i16), - I32 => ( i32::MIN as i128, i32::MAX as i128, cx.tcx.types.i32), - I64 => ( i64::MIN as i128, i64::MAX as i128, cx.tcx.types.i64), - I128 => ( i128::MIN as i128, i128::MAX as i128, cx.tcx.types.i128), + Isize => (isize::MIN as usize as u128, isize::MAX as usize as u128, cx.tcx.types.isize), + I8 => ( i8::MIN as u8 as u128, i8::MAX as u8 as u128, cx.tcx.types.i8), + I16 => ( i16::MIN as u16 as u128, i16::MAX as u16 as u128, cx.tcx.types.i16), + I32 => ( i32::MIN as u32 as u128, i32::MAX as u32 as u128, cx.tcx.types.i32), + I64 => ( i64::MIN as u64 as u128, i64::MAX as u64 as u128, cx.tcx.types.i64), + I128 => ( i128::MIN as u128 as u128, i128::MAX as u128 as u128, cx.tcx.types.i128), }; value_constructors = true; - vec![ConstantRange(ty::Const::from_bits(cx.tcx, unsafe { - transmute::(min) - }, ty), - ty::Const::from_bits(cx.tcx, unsafe { - transmute::(max) - }, ty), + vec![ConstantRange(ty::Const::from_bits(cx.tcx, min, ty), + ty::Const::from_bits(cx.tcx, max, ty), RangeEnd::Included)] } ty::TyUint(uint_ty) if exhaustive_integer_patterns => { @@ -628,6 +624,9 @@ impl<'tcx> Interval<'tcx> { let ty = lo.ty; if let Some(lo) = lo.assert_bits(ty) { if let Some(hi) = hi.assert_bits(ty) { + // Perform a shift if the underlying types are signed, + // which makes the interval arithmetic simpler. + let (lo, hi) = Interval::offset_sign(ty, (lo, hi), true); // Make sure the interval is well-formed. return if lo > hi || lo == hi && *end == RangeEnd::Excluded { None @@ -649,6 +648,38 @@ impl<'tcx> Interval<'tcx> { } } + fn offset_sign(ty: Ty<'tcx>, (lo, hi): (u128, u128), forwards: bool) -> (u128, u128) { + use syntax::ast::IntTy::*; + match ty.sty { + ty::TyInt(int_ty) => { + macro_rules! offset_sign_for_ty { + ($ity:ty, $uty:ty, $min:expr) => {{ + let min = Wrapping($min as $uty); + if forwards { + ((Wrapping(lo as $uty) + min).0 as u128, + (Wrapping(hi as $uty) + min).0 as u128) + } else { + ((Wrapping(lo as $uty) + min).0 as $ity as u128, + (Wrapping(hi as $uty) + min).0 as $ity as u128) + } + }} + } + match int_ty { + Isize => offset_sign_for_ty!(isize, usize, isize::MIN), + I8 => offset_sign_for_ty!(i8, u8, i8::MIN), + I16 => offset_sign_for_ty!(i16, u16, i16::MIN), + I32 => offset_sign_for_ty!(i32, u32, i32::MIN), + I64 => offset_sign_for_ty!(i64, u64, i64::MIN), + I128 => offset_sign_for_ty!(i128, u128, i128::MIN), + } + } + ty::TyUint(_) | ty::TyChar => { + (lo, hi) + } + _ => bug!("`Interval` should only contain integer types") + } + } + fn into_inner(self) -> (u128, u128) { (self.lo, self.hi) } @@ -692,6 +723,7 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // Convert the remaining ranges from pairs to inclusive `ConstantRange`s. let ty = pat_interval.ty; remaining_ranges.into_iter().map(|(lo, hi)| { + let (lo, hi) = Interval::offset_sign(ty, (lo, hi), false); ConstantRange(ty::Const::from_bits(cx.tcx, lo, ty), ty::Const::from_bits(cx.tcx, hi, ty), RangeEnd::Included) From 7476ba4c8aa5b23dde757778bdd153c1bbc869a4 Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 20 May 2018 01:55:05 +0100 Subject: [PATCH 07/47] Add semi-exhaustive tests for exhaustiveness --- src/test/ui/exhaustive_integer_patterns.rs | 105 ++++++++++++++++++ .../ui/exhaustive_integer_patterns.stderr | 39 +++++++ 2 files changed, 144 insertions(+) create mode 100644 src/test/ui/exhaustive_integer_patterns.rs create mode 100644 src/test/ui/exhaustive_integer_patterns.stderr diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs new file mode 100644 index 0000000000000..39bac8919ffdf --- /dev/null +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -0,0 +1,105 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(exhaustive_integer_patterns)] +#![feature(exclusive_range_pattern)] +#![deny(unreachable_patterns)] + +use std::{char, usize, u8, u16, u32, u64, u128, isize, i8, i16, i32, i64, i128}; + +fn main() { + let x: u8 = 0; + + // A single range covering the entire domain. + match x { + 0 ..= 255 => {} // ok + } + + // A combination of ranges and values. + // These are currently allowed to be overlapping. + match x { + 0 ..= 32 => {} + 33 => {} + 34 .. 128 => {} + 100 ..= 200 => {} + 200 => {} //~ ERROR unreachable pattern + 201 ..= 255 => {} + } + + // An incomplete set of values. + match x { //~ ERROR non-exhaustive patterns: `128u8...255u8` not covered + 0 .. 128 => {} + } + + // A more incomplete set of values. + match x { //~ ERROR non-exhaustive patterns + 0 ..= 10 => {} + 20 ..= 30 => {} + 35 => {} + 70 .. 255 => {} + } + + let x: i8 = 0; + match x { //~ ERROR non-exhaustive patterns + -7 => {} + -5..=120 => {} + -2..=20 => {} //~ ERROR unreachable pattern + 125 => {} + } + + // Let's test other types too! + match '\u{0}' { + '\u{0}' ..= char::MAX => {} // ok + } + + match 0usize { + 0 ..= usize::MAX => {} // ok + } + + match 0u16 { + 0 ..= u16::MAX => {} // ok + } + + match 0u32 { + 0 ..= u32::MAX => {} // ok + } + + match 0u64 { + 0 ..= u64::MAX => {} // ok + } + + match 0u128 { + 0 ..= u128::MAX => {} // ok + } + + match 0isize { + isize::MIN ..= isize::MAX => {} // ok + } + + match 0i8 { + -128..=127 => {} // ok + } + + match 0i16 { + i16::MIN ..= i16::MAX => {} // ok + } + + match 0i32 { + i32::MIN ..= i32::MAX => {} // ok + } + + match 0i64 { + i64::MIN ..= i64::MAX => {} // ok + } + + match 0i128 { + i128::MIN ..= i128::MAX => {} // ok + } +} diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr new file mode 100644 index 0000000000000..5a0bdebde1f34 --- /dev/null +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -0,0 +1,39 @@ +error: unreachable pattern + --> $DIR/exhaustive_integer_patterns.rs:32:9 + | +LL | 200 => {} //~ ERROR unreachable pattern + | ^^^ + | +note: lint level defined here + --> $DIR/exhaustive_integer_patterns.rs:13:9 + | +LL | #![deny(unreachable_patterns)] + | ^^^^^^^^^^^^^^^^^^^^ + +error[E0004]: non-exhaustive patterns: `128u8...255u8` not covered + --> $DIR/exhaustive_integer_patterns.rs:37:11 + | +LL | match x { //~ ERROR non-exhaustive patterns: `128u8...255u8` not covered + | ^ pattern `128u8...255u8` not covered + +error[E0004]: non-exhaustive patterns: `11u8...20u8`, `30u8...35u8`, `35u8...70u8` and 1 more not covered + --> $DIR/exhaustive_integer_patterns.rs:42:11 + | +LL | match x { //~ ERROR non-exhaustive patterns + | ^ patterns `11u8...20u8`, `30u8...35u8`, `35u8...70u8` and 1 more not covered + +error: unreachable pattern + --> $DIR/exhaustive_integer_patterns.rs:53:9 + | +LL | -2..=20 => {} //~ ERROR unreachable pattern + | ^^^^^^^ + +error[E0004]: non-exhaustive patterns: `-128i8...-5i8`, `120i8...121i8` and `121i8...127i8` not covered + --> $DIR/exhaustive_integer_patterns.rs:50:11 + | +LL | match x { //~ ERROR non-exhaustive patterns + | ^ patterns `-128i8...-5i8`, `120i8...121i8` and `121i8...127i8` not covered + +error: aborting due to 5 previous errors + +For more information about this error, try `rustc --explain E0004`. From 9778a81e9287571710ebbd83aba1a6b816297c3b Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 20 May 2018 02:10:37 +0100 Subject: [PATCH 08/47] Improve macros with reduced repetition --- src/librustc_mir/hair/pattern/_match.rs | 35 ++++++++++++++----------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 03b1ea44c753d..c7deb30dd3f5c 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -273,7 +273,7 @@ impl<'tcx> Constructor<'tcx> { } } -#[derive(Clone, Debug)] +#[derive(Clone)] pub enum Usefulness<'tcx> { Useful, UsefulWithWitness(Vec>), @@ -468,13 +468,18 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } ty::TyInt(int_ty) if exhaustive_integer_patterns => { use syntax::ast::IntTy::*; + macro_rules! min_max_ty { + ($ity:ident, $uty:ty, $sty:expr) => { + ($ity::MIN as $uty as u128, $ity::MAX as $uty as u128, $sty) + } + } let (min, max, ty) = match int_ty { - Isize => (isize::MIN as usize as u128, isize::MAX as usize as u128, cx.tcx.types.isize), - I8 => ( i8::MIN as u8 as u128, i8::MAX as u8 as u128, cx.tcx.types.i8), - I16 => ( i16::MIN as u16 as u128, i16::MAX as u16 as u128, cx.tcx.types.i16), - I32 => ( i32::MIN as u32 as u128, i32::MAX as u32 as u128, cx.tcx.types.i32), - I64 => ( i64::MIN as u64 as u128, i64::MAX as u64 as u128, cx.tcx.types.i64), - I128 => ( i128::MIN as u128 as u128, i128::MAX as u128 as u128, cx.tcx.types.i128), + Isize => min_max_ty!(isize, usize, cx.tcx.types.isize), + I8 => min_max_ty!(i8, u8, cx.tcx.types.i8), + I16 => min_max_ty!(i16, u16, cx.tcx.types.i16), + I32 => min_max_ty!(i32, u32, cx.tcx.types.i32), + I64 => min_max_ty!(i64, u64, cx.tcx.types.i64), + I128 => min_max_ty!(i128, u128, cx.tcx.types.i128), }; value_constructors = true; vec![ConstantRange(ty::Const::from_bits(cx.tcx, min, ty), @@ -653,8 +658,8 @@ impl<'tcx> Interval<'tcx> { match ty.sty { ty::TyInt(int_ty) => { macro_rules! offset_sign_for_ty { - ($ity:ty, $uty:ty, $min:expr) => {{ - let min = Wrapping($min as $uty); + ($ity:ident, $uty:ty) => {{ + let min = Wrapping($ity::MIN as $uty); if forwards { ((Wrapping(lo as $uty) + min).0 as u128, (Wrapping(hi as $uty) + min).0 as u128) @@ -665,12 +670,12 @@ impl<'tcx> Interval<'tcx> { }} } match int_ty { - Isize => offset_sign_for_ty!(isize, usize, isize::MIN), - I8 => offset_sign_for_ty!(i8, u8, i8::MIN), - I16 => offset_sign_for_ty!(i16, u16, i16::MIN), - I32 => offset_sign_for_ty!(i32, u32, i32::MIN), - I64 => offset_sign_for_ty!(i64, u64, i64::MIN), - I128 => offset_sign_for_ty!(i128, u128, i128::MIN), + Isize => offset_sign_for_ty!(isize, usize), + I8 => offset_sign_for_ty!(i8, u8), + I16 => offset_sign_for_ty!(i16, u16), + I32 => offset_sign_for_ty!(i32, u32), + I64 => offset_sign_for_ty!(i64, u64), + I128 => offset_sign_for_ty!(i128, u128), } } ty::TyUint(_) | ty::TyChar => { From a20cb1084a5a90cf76ca3207d42b16b9c3ab1c76 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 21 May 2018 20:40:38 +0100 Subject: [PATCH 09/47] Require just the Unicode Scalar Values to be matched for a char --- src/librustc_mir/hair/pattern/_match.rs | 12 ++++++++---- src/test/ui/exhaustive_integer_patterns.rs | 13 +++++++++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index c7deb30dd3f5c..4a94030e87605 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -460,11 +460,15 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, .collect() } ty::TyChar if exhaustive_integer_patterns => { - let (min, max) = (0u128, char::MAX as u128); + let endpoint = |c: char| { + ty::Const::from_bits(cx.tcx, c as u128, cx.tcx.types.char) + }; value_constructors = true; - vec![ConstantRange(ty::Const::from_bits(cx.tcx, min, cx.tcx.types.char), - ty::Const::from_bits(cx.tcx, max, cx.tcx.types.char), - RangeEnd::Included)] + vec![ + // The valid Unicode Scalar Value ranges. + ConstantRange(endpoint('\u{0000}'), endpoint('\u{D7FF}'), RangeEnd::Included), + ConstantRange(endpoint('\u{E000}'), endpoint('\u{10FFFF}'), RangeEnd::Included), + ] } ty::TyInt(int_ty) if exhaustive_integer_patterns => { use syntax::ast::IntTy::*; diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs index 39bac8919ffdf..8076441504565 100644 --- a/src/test/ui/exhaustive_integer_patterns.rs +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -55,10 +55,19 @@ fn main() { } // Let's test other types too! - match '\u{0}' { + let c: char = '\u{0}'; + match c { '\u{0}' ..= char::MAX => {} // ok } + // We can actually get away with just covering the + // following two ranges, which correspond to all + // valid Unicode Scalar Values. + match c { + '\u{0000}' ..= '\u{D7FF}' => {} + '\u{E000}' ..= '\u{10_FFFF}' => {} + } + match 0usize { 0 ..= usize::MAX => {} // ok } @@ -84,7 +93,7 @@ fn main() { } match 0i8 { - -128..=127 => {} // ok + -128 ..= 127 => {} // ok } match 0i16 { From 7f720304218da4d671bcbc332c0342f9405962c8 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 21 May 2018 20:50:40 +0100 Subject: [PATCH 10/47] Fix range splitting --- src/librustc_mir/hair/pattern/_match.rs | 4 ++-- src/test/ui/exhaustive_integer_patterns.stderr | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 4a94030e87605..eae2bd7a40422 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -725,8 +725,8 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } else { // The pattern intersects the middle of the subrange, // so we create two ranges either side of the intersection.) - remaining_ranges.push((subrange_lo, pat_interval.lo)); - remaining_ranges.push((pat_interval.hi, subrange_hi)); + remaining_ranges.push((subrange_lo, pat_interval.lo - 1)); + remaining_ranges.push((pat_interval.hi + 1, subrange_hi)); } } // Convert the remaining ranges from pairs to inclusive `ConstantRange`s. diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr index 5a0bdebde1f34..85f12dad04736 100644 --- a/src/test/ui/exhaustive_integer_patterns.stderr +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -16,11 +16,11 @@ error[E0004]: non-exhaustive patterns: `128u8...255u8` not covered LL | match x { //~ ERROR non-exhaustive patterns: `128u8...255u8` not covered | ^ pattern `128u8...255u8` not covered -error[E0004]: non-exhaustive patterns: `11u8...20u8`, `30u8...35u8`, `35u8...70u8` and 1 more not covered +error[E0004]: non-exhaustive patterns: `11u8...19u8`, `31u8...34u8`, `36u8...69u8` and 1 more not covered --> $DIR/exhaustive_integer_patterns.rs:42:11 | LL | match x { //~ ERROR non-exhaustive patterns - | ^ patterns `11u8...20u8`, `30u8...35u8`, `35u8...70u8` and 1 more not covered + | ^ patterns `11u8...19u8`, `31u8...34u8`, `36u8...69u8` and 1 more not covered error: unreachable pattern --> $DIR/exhaustive_integer_patterns.rs:53:9 @@ -28,11 +28,11 @@ error: unreachable pattern LL | -2..=20 => {} //~ ERROR unreachable pattern | ^^^^^^^ -error[E0004]: non-exhaustive patterns: `-128i8...-5i8`, `120i8...121i8` and `121i8...127i8` not covered +error[E0004]: non-exhaustive patterns: `-128i8...-6i8` and `122i8...127i8` not covered --> $DIR/exhaustive_integer_patterns.rs:50:11 | LL | match x { //~ ERROR non-exhaustive patterns - | ^ patterns `-128i8...-5i8`, `120i8...121i8` and `121i8...127i8` not covered + | ^ patterns `-128i8...-6i8` and `122i8...127i8` not covered error: aborting due to 5 previous errors From c00fd8f58c9c7780b83fbb4e5e2e419f1ddd37e2 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 21 May 2018 21:46:12 +0100 Subject: [PATCH 11/47] Refactor interval conditions --- src/librustc_mir/hair/pattern/_match.rs | 43 +++++++++++-------------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index eae2bd7a40422..204d3ea8c9c20 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -620,8 +620,7 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( /// An inclusive interval, used for precise integer exhaustiveness checking. struct Interval<'tcx> { - pub lo: u128, - pub hi: u128, + pub range: RangeInclusive, pub ty: Ty<'tcx>, } @@ -641,7 +640,7 @@ impl<'tcx> Interval<'tcx> { None } else { let offset = (*end == RangeEnd::Excluded) as u128; - Some(Interval { lo, hi: hi - offset, ty }) + Some(Interval { range: lo..=(hi - offset), ty }) }; } } @@ -649,7 +648,7 @@ impl<'tcx> Interval<'tcx> { } ConstantValue(val) => { let ty = val.ty; - val.assert_bits(ty).map(|val| Interval { lo: val, hi: val, ty }) + val.assert_bits(ty).map(|val| Interval { range: val..=val, ty }) } Single | Variant(_) | Slice(_) => { None @@ -690,7 +689,7 @@ impl<'tcx> Interval<'tcx> { } fn into_inner(self) -> (u128, u128) { - (self.lo, self.hi) + self.range.into_inner() } } @@ -706,31 +705,27 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let mut ranges: Vec<_> = ranges.into_iter().filter_map(|r| { Interval::from_ctor(&r).map(|i| i.into_inner()) }).collect(); + let ty = pat_interval.ty; + let (pat_interval_lo, pat_interval_hi) = pat_interval.into_inner(); for (subrange_lo, subrange_hi) in ranges { - if pat_interval.lo > subrange_hi || pat_interval.hi < subrange_lo { + if pat_interval_lo > subrange_hi || subrange_lo > pat_interval_hi { // The pattern doesn't intersect with the subrange at all, // so the subrange remains untouched. remaining_ranges.push((subrange_lo, subrange_hi)); - } else if pat_interval.lo <= subrange_lo && pat_interval.hi >= subrange_hi { - // The pattern entirely covers the subrange of values, - // so we no longer have to consider this subrange_ - } else if pat_interval.lo <= subrange_lo { - // The pattern intersects a lower section of the subrange, - // so only the upper section will remain. - remaining_ranges.push((pat_interval.hi + 1, subrange_hi)); - } else if pat_interval.hi >= subrange_hi { - // The pattern intersects an upper section of the subrange, - // so only the lower section will remain. - remaining_ranges.push((subrange_lo, pat_interval.lo - 1)); } else { - // The pattern intersects the middle of the subrange, - // so we create two ranges either side of the intersection.) - remaining_ranges.push((subrange_lo, pat_interval.lo - 1)); - remaining_ranges.push((pat_interval.hi + 1, subrange_hi)); + if pat_interval_lo > subrange_lo { + // The pattern intersects an upper section of the + // subrange, so a lower section will remain. + remaining_ranges.push((subrange_lo, pat_interval_lo - 1)); + } + if pat_interval_hi < subrange_hi { + // The pattern intersects a lower section of the + // subrange, so an upper section will remain. + remaining_ranges.push((pat_interval_hi + 1, subrange_hi)); + } } } // Convert the remaining ranges from pairs to inclusive `ConstantRange`s. - let ty = pat_interval.ty; remaining_ranges.into_iter().map(|(lo, hi)| { let (lo, hi) = Interval::offset_sign(ty, (lo, hi), false); ConstantRange(ty::Const::from_bits(cx.tcx, lo, ty), @@ -839,7 +834,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // `missing_ctors` are those that should have appeared // as patterns in the `match` expression, but did not. let mut missing_ctors = vec![]; - 'req: for req_ctor in all_ctors.clone() { + 'req: for req_ctor in &all_ctors { let mut sub_ctors = vec![req_ctor.clone()]; // The only constructor patterns for which it is valid to // treat the values as constructors are ranges (see @@ -861,7 +856,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // If the pattern for the required constructor // appears in the `match`, then it is not missing, // and we can move on to the next one. - if *used_ctor == req_ctor { + if used_ctor == req_ctor { continue 'req; } } From 7695bd0be9cc6d08b9d58c1c0ef193458857ece7 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 21 May 2018 23:51:01 +0100 Subject: [PATCH 12/47] Use bit operators for min_max_ty --- src/librustc_mir/hair/pattern/_match.rs | 46 +++++++++++++++---------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 204d3ea8c9c20..9f0f07f2a9ce9 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -472,18 +472,20 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } ty::TyInt(int_ty) if exhaustive_integer_patterns => { use syntax::ast::IntTy::*; - macro_rules! min_max_ty { - ($ity:ident, $uty:ty, $sty:expr) => { - ($ity::MIN as $uty as u128, $ity::MAX as $uty as u128, $sty) - } - } + let min_max_ty = |sty| { + let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(sty)) + .unwrap().size.bits() as i128; + let min = -(1 << (size - 1)); + let max = (1 << (size - 1)) - 1; + (min as u128, max as u128, sty) + }; let (min, max, ty) = match int_ty { - Isize => min_max_ty!(isize, usize, cx.tcx.types.isize), - I8 => min_max_ty!(i8, u8, cx.tcx.types.i8), - I16 => min_max_ty!(i16, u16, cx.tcx.types.i16), - I32 => min_max_ty!(i32, u32, cx.tcx.types.i32), - I64 => min_max_ty!(i64, u64, cx.tcx.types.i64), - I128 => min_max_ty!(i128, u128, cx.tcx.types.i128), + Isize => min_max_ty(cx.tcx.types.isize), + I8 => min_max_ty(cx.tcx.types.i8), + I16 => min_max_ty(cx.tcx.types.i16), + I32 => min_max_ty(cx.tcx.types.i32), + I64 => min_max_ty(cx.tcx.types.i64), + I128 => min_max_ty(cx.tcx.types.i128), }; value_constructors = true; vec![ConstantRange(ty::Const::from_bits(cx.tcx, min, ty), @@ -492,14 +494,20 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } ty::TyUint(uint_ty) if exhaustive_integer_patterns => { use syntax::ast::UintTy::*; - let (min, (max, ty)) = (0u128, match uint_ty { - Usize => (usize::MAX as u128, cx.tcx.types.usize), - U8 => ( u8::MAX as u128, cx.tcx.types.u8), - U16 => ( u16::MAX as u128, cx.tcx.types.u16), - U32 => ( u32::MAX as u128, cx.tcx.types.u32), - U64 => ( u64::MAX as u128, cx.tcx.types.u64), - U128 => ( u128::MAX as u128, cx.tcx.types.u128), - }); + let min_max_ty = |sty| { + let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(sty)) + .unwrap().size.bits() as i128; + let max = (1 << size) - 1; + (0u128, max as u128, sty) + }; + let (min, max, ty) = match uint_ty { + Usize => min_max_ty(cx.tcx.types.usize), + U8 => min_max_ty(cx.tcx.types.u8), + U16 => min_max_ty(cx.tcx.types.u16), + U32 => min_max_ty(cx.tcx.types.u32), + U64 => min_max_ty(cx.tcx.types.u64), + U128 => min_max_ty(cx.tcx.types.u128), + }; value_constructors = true; vec![ConstantRange(ty::Const::from_bits(cx.tcx, min, ty), ty::Const::from_bits(cx.tcx, max, ty), From a553fa724441a7b810980eada750a0cdf843f286 Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 22 May 2018 01:02:47 +0100 Subject: [PATCH 13/47] Fix integer overflow --- src/librustc_mir/hair/pattern/_match.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 9f0f07f2a9ce9..3f483dfee55b5 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -475,8 +475,8 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let min_max_ty = |sty| { let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(sty)) .unwrap().size.bits() as i128; - let min = -(1 << (size - 1)); - let max = (1 << (size - 1)) - 1; + let min = (1i128 << (size - 1)).wrapping_neg(); + let max = (1i128 << (size - 1)).wrapping_sub(1); (min as u128, max as u128, sty) }; let (min, max, ty) = match int_ty { @@ -496,8 +496,9 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, use syntax::ast::UintTy::*; let min_max_ty = |sty| { let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(sty)) - .unwrap().size.bits() as i128; - let max = (1 << size) - 1; + .unwrap().size.bits() as u32; + let shift = 1u128.overflowing_shl(size); + let max = shift.0.wrapping_sub(1 + (shift.1 as u128)); (0u128, max as u128, sty) }; let (min, max, ty) = match uint_ty { From 838997236e7c91e706f8d9f1f884d958c1bcaee7 Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 22 May 2018 01:03:05 +0100 Subject: [PATCH 14/47] Add singleton patterns to test --- src/test/ui/exhaustive_integer_patterns.rs | 11 ++++++++++- src/test/ui/exhaustive_integer_patterns.stderr | 16 ++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs index 8076441504565..e8f2affbec358 100644 --- a/src/test/ui/exhaustive_integer_patterns.rs +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -34,7 +34,7 @@ fn main() { } // An incomplete set of values. - match x { //~ ERROR non-exhaustive patterns: `128u8...255u8` not covered + match x { //~ ERROR non-exhaustive patterns 0 .. 128 => {} } @@ -96,10 +96,19 @@ fn main() { -128 ..= 127 => {} // ok } + match 0i8 { //~ ERROR non-exhaustive patterns + -127 ..= 127 => {} + } + match 0i16 { i16::MIN ..= i16::MAX => {} // ok } + match 0i16 { //~ ERROR non-exhaustive patterns + i16::MIN ..= -1 => {} + 1 ..= i16::MAX => {} + } + match 0i32 { i32::MIN ..= i32::MAX => {} // ok } diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr index 85f12dad04736..243ba05251a48 100644 --- a/src/test/ui/exhaustive_integer_patterns.stderr +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -13,7 +13,7 @@ LL | #![deny(unreachable_patterns)] error[E0004]: non-exhaustive patterns: `128u8...255u8` not covered --> $DIR/exhaustive_integer_patterns.rs:37:11 | -LL | match x { //~ ERROR non-exhaustive patterns: `128u8...255u8` not covered +LL | match x { //~ ERROR non-exhaustive patterns | ^ pattern `128u8...255u8` not covered error[E0004]: non-exhaustive patterns: `11u8...19u8`, `31u8...34u8`, `36u8...69u8` and 1 more not covered @@ -34,6 +34,18 @@ error[E0004]: non-exhaustive patterns: `-128i8...-6i8` and `122i8...127i8` not c LL | match x { //~ ERROR non-exhaustive patterns | ^ patterns `-128i8...-6i8` and `122i8...127i8` not covered -error: aborting due to 5 previous errors +error[E0004]: non-exhaustive patterns: `-128i8` not covered + --> $DIR/exhaustive_integer_patterns.rs:99:11 + | +LL | match 0i8 { //~ ERROR non-exhaustive patterns + | ^^^ pattern `-128i8` not covered + +error[E0004]: non-exhaustive patterns: `0i16` not covered + --> $DIR/exhaustive_integer_patterns.rs:107:11 + | +LL | match 0i16 { //~ ERROR non-exhaustive patterns + | ^^^^ pattern `0i16` not covered + +error: aborting due to 7 previous errors For more information about this error, try `rustc --explain E0004`. From f4af3b015f7192d4dab33b8d4ee984b7b258037e Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 22 May 2018 18:23:30 +0100 Subject: [PATCH 15/47] Refactor to remove explicit integer type matching --- src/librustc_mir/hair/pattern/_match.rs | 68 ++++++++----------------- 1 file changed, 22 insertions(+), 46 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 3f483dfee55b5..281dc624a62aa 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -138,7 +138,6 @@ impl<'a, 'tcx> FromIterator>> for Matrix<'a, 'tcx> { } } -//NOTE: appears to be the only place other then InferCtxt to contain a ParamEnv pub struct MatchCheckCtxt<'a, 'tcx: 'a> { pub tcx: TyCtxt<'a, 'tcx, 'tcx>, /// The module in which the match occurs. This is necessary for @@ -470,48 +469,24 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ConstantRange(endpoint('\u{E000}'), endpoint('\u{10FFFF}'), RangeEnd::Included), ] } - ty::TyInt(int_ty) if exhaustive_integer_patterns => { - use syntax::ast::IntTy::*; - let min_max_ty = |sty| { - let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(sty)) - .unwrap().size.bits() as i128; - let min = (1i128 << (size - 1)).wrapping_neg(); - let max = (1i128 << (size - 1)).wrapping_sub(1); - (min as u128, max as u128, sty) - }; - let (min, max, ty) = match int_ty { - Isize => min_max_ty(cx.tcx.types.isize), - I8 => min_max_ty(cx.tcx.types.i8), - I16 => min_max_ty(cx.tcx.types.i16), - I32 => min_max_ty(cx.tcx.types.i32), - I64 => min_max_ty(cx.tcx.types.i64), - I128 => min_max_ty(cx.tcx.types.i128), - }; + ty::TyInt(_) if exhaustive_integer_patterns => { + let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) + .unwrap().size.bits() as i128; + let min = (1i128 << (size - 1)).wrapping_neg(); + let max = (1i128 << (size - 1)).wrapping_sub(1); value_constructors = true; - vec![ConstantRange(ty::Const::from_bits(cx.tcx, min, ty), - ty::Const::from_bits(cx.tcx, max, ty), + vec![ConstantRange(ty::Const::from_bits(cx.tcx, min as u128, pcx.ty), + ty::Const::from_bits(cx.tcx, max as u128, pcx.ty), RangeEnd::Included)] } - ty::TyUint(uint_ty) if exhaustive_integer_patterns => { - use syntax::ast::UintTy::*; - let min_max_ty = |sty| { - let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(sty)) - .unwrap().size.bits() as u32; - let shift = 1u128.overflowing_shl(size); - let max = shift.0.wrapping_sub(1 + (shift.1 as u128)); - (0u128, max as u128, sty) - }; - let (min, max, ty) = match uint_ty { - Usize => min_max_ty(cx.tcx.types.usize), - U8 => min_max_ty(cx.tcx.types.u8), - U16 => min_max_ty(cx.tcx.types.u16), - U32 => min_max_ty(cx.tcx.types.u32), - U64 => min_max_ty(cx.tcx.types.u64), - U128 => min_max_ty(cx.tcx.types.u128), - }; + ty::TyUint(_) if exhaustive_integer_patterns => { + let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) + .unwrap().size.bits() as u32; + let shift = 1u128.overflowing_shl(size); + let max = shift.0.wrapping_sub(1 + (shift.1 as u128)); value_constructors = true; - vec![ConstantRange(ty::Const::from_bits(cx.tcx, min, ty), - ty::Const::from_bits(cx.tcx, max, ty), + vec![ConstantRange(ty::Const::from_bits(cx.tcx, 0u128, pcx.ty), + ty::Const::from_bits(cx.tcx, max as u128, pcx.ty), RangeEnd::Included)] } _ => { @@ -643,7 +618,7 @@ impl<'tcx> Interval<'tcx> { if let Some(hi) = hi.assert_bits(ty) { // Perform a shift if the underlying types are signed, // which makes the interval arithmetic simpler. - let (lo, hi) = Interval::offset_sign(ty, (lo, hi), true); + let (lo, hi) = Interval::offset_sign(ty, lo..=hi, true); // Make sure the interval is well-formed. return if lo > hi || lo == hi && *end == RangeEnd::Excluded { None @@ -665,7 +640,8 @@ impl<'tcx> Interval<'tcx> { } } - fn offset_sign(ty: Ty<'tcx>, (lo, hi): (u128, u128), forwards: bool) -> (u128, u128) { + fn offset_sign(ty: Ty<'tcx>, range: RangeInclusive, forwards: bool) -> (u128, u128) { + let (lo, hi) = range.into_inner(); use syntax::ast::IntTy::*; match ty.sty { ty::TyInt(int_ty) => { @@ -720,23 +696,23 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, if pat_interval_lo > subrange_hi || subrange_lo > pat_interval_hi { // The pattern doesn't intersect with the subrange at all, // so the subrange remains untouched. - remaining_ranges.push((subrange_lo, subrange_hi)); + remaining_ranges.push(subrange_lo..=subrange_hi); } else { if pat_interval_lo > subrange_lo { // The pattern intersects an upper section of the // subrange, so a lower section will remain. - remaining_ranges.push((subrange_lo, pat_interval_lo - 1)); + remaining_ranges.push(subrange_lo..=(pat_interval_lo - 1)); } if pat_interval_hi < subrange_hi { // The pattern intersects a lower section of the // subrange, so an upper section will remain. - remaining_ranges.push((pat_interval_hi + 1, subrange_hi)); + remaining_ranges.push((pat_interval_hi + 1)..=subrange_hi); } } } // Convert the remaining ranges from pairs to inclusive `ConstantRange`s. - remaining_ranges.into_iter().map(|(lo, hi)| { - let (lo, hi) = Interval::offset_sign(ty, (lo, hi), false); + remaining_ranges.into_iter().map(|r| { + let (lo, hi) = Interval::offset_sign(ty, r, false); ConstantRange(ty::Const::from_bits(cx.tcx, lo, ty), ty::Const::from_bits(cx.tcx, hi, ty), RangeEnd::Included) From a9f2c5a7b27e929061117095ae6f2ec5eb2a1b4d Mon Sep 17 00:00:00 2001 From: varkor Date: Wed, 23 May 2018 10:22:45 +0100 Subject: [PATCH 16/47] Fix sign conversion arithmetic errors --- src/librustc_mir/hair/pattern/_match.rs | 73 +++++++++++-------- .../ui/exhaustive_integer_patterns.stderr | 4 +- 2 files changed, 44 insertions(+), 33 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 281dc624a62aa..ceb065cc0c877 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -471,9 +471,9 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } ty::TyInt(_) if exhaustive_integer_patterns => { let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) - .unwrap().size.bits() as i128; - let min = (1i128 << (size - 1)).wrapping_neg(); - let max = (1i128 << (size - 1)).wrapping_sub(1); + .unwrap().size.bits() as u128; + let min = (1u128 << (size - 1)).wrapping_neg(); + let max = (1u128 << (size - 1)).wrapping_sub(1); value_constructors = true; vec![ConstantRange(ty::Const::from_bits(cx.tcx, min as u128, pcx.ty), ty::Const::from_bits(cx.tcx, max as u128, pcx.ty), @@ -603,13 +603,17 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( } /// An inclusive interval, used for precise integer exhaustiveness checking. +/// `Interval`s always store a contiguous range of integers. That means that +/// signed integers are offset (see `offset_sign`) by their minimum value. struct Interval<'tcx> { pub range: RangeInclusive, pub ty: Ty<'tcx>, } impl<'tcx> Interval<'tcx> { - fn from_ctor(ctor: &Constructor<'tcx>) -> Option> { + fn from_ctor(tcx: TyCtxt<'_, 'tcx, 'tcx>, + ctor: &Constructor<'tcx>) + -> Option> { match ctor { ConstantRange(lo, hi, end) => { assert_eq!(lo.ty, hi.ty); @@ -618,7 +622,7 @@ impl<'tcx> Interval<'tcx> { if let Some(hi) = hi.assert_bits(ty) { // Perform a shift if the underlying types are signed, // which makes the interval arithmetic simpler. - let (lo, hi) = Interval::offset_sign(ty, lo..=hi, true); + let (lo, hi) = Self::offset_sign(tcx, ty, lo..=hi, true); // Make sure the interval is well-formed. return if lo > hi || lo == hi && *end == RangeEnd::Excluded { None @@ -632,7 +636,12 @@ impl<'tcx> Interval<'tcx> { } ConstantValue(val) => { let ty = val.ty; - val.assert_bits(ty).map(|val| Interval { range: val..=val, ty }) + if let Some(val) = val.assert_bits(ty) { + let (lo, hi) = Self::offset_sign(tcx, ty, val..=val, true); + Some(Interval { range: lo..=hi, ty }) + } else { + None + } } Single | Variant(_) | Slice(_) => { None @@ -640,30 +649,32 @@ impl<'tcx> Interval<'tcx> { } } - fn offset_sign(ty: Ty<'tcx>, range: RangeInclusive, forwards: bool) -> (u128, u128) { + fn offset_sign(tcx: TyCtxt<'_, 'tcx, 'tcx>, + ty: Ty<'tcx>, + range: RangeInclusive, + encode: bool) + -> (u128, u128) { + // We ensure that all integer values are contiguous: that is, that their + // minimum value is represented by 0, so that comparisons and increments/ + // decrements on interval endpoints work consistently whether the endpoints + // are signed or unsigned. let (lo, hi) = range.into_inner(); - use syntax::ast::IntTy::*; match ty.sty { - ty::TyInt(int_ty) => { - macro_rules! offset_sign_for_ty { - ($ity:ident, $uty:ty) => {{ - let min = Wrapping($ity::MIN as $uty); - if forwards { - ((Wrapping(lo as $uty) + min).0 as u128, - (Wrapping(hi as $uty) + min).0 as u128) - } else { - ((Wrapping(lo as $uty) + min).0 as $ity as u128, - (Wrapping(hi as $uty) + min).0 as $ity as u128) - } - }} - } - match int_ty { - Isize => offset_sign_for_ty!(isize, usize), - I8 => offset_sign_for_ty!(i8, u8), - I16 => offset_sign_for_ty!(i16, u16), - I32 => offset_sign_for_ty!(i32, u32), - I64 => offset_sign_for_ty!(i64, u64), - I128 => offset_sign_for_ty!(i128, u128), + ty::TyInt(_) => { + let size = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) + .unwrap().size.bits() as u128; + let min = (1u128 << (size - 1)).wrapping_neg(); + let shift = 1u128.overflowing_shl(size as u32); + let mask = shift.0.wrapping_sub(1 + (shift.1 as u128)); + if encode { + let offset = |x: u128| x.wrapping_sub(min) & mask; + (offset(lo), offset(hi)) + } else { + let offset = |x: u128| { + interpret::sign_extend(tcx, x.wrapping_add(min) & mask, ty) + .expect("layout error for TyInt") + }; + (offset(lo), offset(hi)) } } ty::TyUint(_) | ty::TyChar => { @@ -685,10 +696,10 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, pat_ctor: &Constructor<'tcx>, ranges: Vec>) -> Vec> { - if let Some(pat_interval) = Interval::from_ctor(pat_ctor) { + if let Some(pat_interval) = Interval::from_ctor(cx.tcx, pat_ctor) { let mut remaining_ranges = vec![]; let mut ranges: Vec<_> = ranges.into_iter().filter_map(|r| { - Interval::from_ctor(&r).map(|i| i.into_inner()) + Interval::from_ctor(cx.tcx, &r).map(|i| i.into_inner()) }).collect(); let ty = pat_interval.ty; let (pat_interval_lo, pat_interval_hi) = pat_interval.into_inner(); @@ -712,7 +723,7 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } // Convert the remaining ranges from pairs to inclusive `ConstantRange`s. remaining_ranges.into_iter().map(|r| { - let (lo, hi) = Interval::offset_sign(ty, r, false); + let (lo, hi) = Interval::offset_sign(cx.tcx, ty, r, false); ConstantRange(ty::Const::from_bits(cx.tcx, lo, ty), ty::Const::from_bits(cx.tcx, hi, ty), RangeEnd::Included) diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr index 243ba05251a48..3d308a9e5d1e6 100644 --- a/src/test/ui/exhaustive_integer_patterns.stderr +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -28,11 +28,11 @@ error: unreachable pattern LL | -2..=20 => {} //~ ERROR unreachable pattern | ^^^^^^^ -error[E0004]: non-exhaustive patterns: `-128i8...-6i8` and `122i8...127i8` not covered +error[E0004]: non-exhaustive patterns: `-128i8...-8i8`, `-6i8`, `121i8...124i8` and 1 more not covered --> $DIR/exhaustive_integer_patterns.rs:50:11 | LL | match x { //~ ERROR non-exhaustive patterns - | ^ patterns `-128i8...-6i8` and `122i8...127i8` not covered + | ^ patterns `-128i8...-8i8`, `-6i8`, `121i8...124i8` and 1 more not covered error[E0004]: non-exhaustive patterns: `-128i8` not covered --> $DIR/exhaustive_integer_patterns.rs:99:11 From effb3d05a056698df709f977c29c31dc702f87bd Mon Sep 17 00:00:00 2001 From: varkor Date: Wed, 23 May 2018 21:16:07 +0100 Subject: [PATCH 17/47] Improve the comments --- src/librustc_mir/hair/pattern/_match.rs | 28 +++++++++++++++---------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index ceb065cc0c877..1c9cdfd03b390 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -470,20 +470,21 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ] } ty::TyInt(_) if exhaustive_integer_patterns => { - let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) + // FIXME(49937): refactor these bit manipulations into interpret. + let bits = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) .unwrap().size.bits() as u128; - let min = (1u128 << (size - 1)).wrapping_neg(); - let max = (1u128 << (size - 1)).wrapping_sub(1); + let min = 1u128 << (bits - 1); + let max = (1u128 << (bits - 1)) - 1; value_constructors = true; vec![ConstantRange(ty::Const::from_bits(cx.tcx, min as u128, pcx.ty), ty::Const::from_bits(cx.tcx, max as u128, pcx.ty), RangeEnd::Included)] } ty::TyUint(_) if exhaustive_integer_patterns => { - let size = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) + // FIXME(49937): refactor these bit manipulations into interpret. + let bits = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) .unwrap().size.bits() as u32; - let shift = 1u128.overflowing_shl(size); - let max = shift.0.wrapping_sub(1 + (shift.1 as u128)); + let max = (!0u128).wrapping_shr(128 - bits); value_constructors = true; vec![ConstantRange(ty::Const::from_bits(cx.tcx, 0u128, pcx.ty), ty::Const::from_bits(cx.tcx, max as u128, pcx.ty), @@ -603,8 +604,12 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( } /// An inclusive interval, used for precise integer exhaustiveness checking. -/// `Interval`s always store a contiguous range of integers. That means that -/// signed integers are offset (see `offset_sign`) by their minimum value. +/// `Interval`s always store a contiguous range of integers. This means that +/// signed values are encoded by offsetting them such that `0` represents the +/// minimum value for the integer, regardless of sign. +/// For example, the range `-128...127` is encoded as `0...255`. +/// This makes comparisons and arithmetic on interval endpoints much more +/// straightforward. See `offset_sign` for the conversion technique. struct Interval<'tcx> { pub range: RangeInclusive, pub ty: Ty<'tcx>, @@ -661,10 +666,11 @@ impl<'tcx> Interval<'tcx> { let (lo, hi) = range.into_inner(); match ty.sty { ty::TyInt(_) => { - let size = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) + // FIXME(49937): refactor these bit manipulations into interpret. + let bits = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) .unwrap().size.bits() as u128; - let min = (1u128 << (size - 1)).wrapping_neg(); - let shift = 1u128.overflowing_shl(size as u32); + let min = 1u128 << (bits - 1); + let shift = 1u128.overflowing_shl(bits as u32); let mask = shift.0.wrapping_sub(1 + (shift.1 as u128)); if encode { let offset = |x: u128| x.wrapping_sub(min) & mask; From c388c11a60213152229b43b389a81ee828591fc0 Mon Sep 17 00:00:00 2001 From: varkor Date: Wed, 23 May 2018 22:09:50 +0100 Subject: [PATCH 18/47] Special-case (RangeEnd::Included, Ordering::Equal) in lower_pattern_unadjusted --- src/librustc_mir/hair/pattern/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/hair/pattern/mod.rs b/src/librustc_mir/hair/pattern/mod.rs index d614131c52683..6e56db69a823f 100644 --- a/src/librustc_mir/hair/pattern/mod.rs +++ b/src/librustc_mir/hair/pattern/mod.rs @@ -391,7 +391,12 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { err.emit(); PatternKind::Wild }, - (RangeEnd::Included, Some(_)) => PatternKind::Range { lo, hi, end }, + (RangeEnd::Included, Some(Ordering::Equal)) => { + PatternKind::Constant { value: lo } + } + (RangeEnd::Included, Some(Ordering::Less)) => { + PatternKind::Range { lo, hi, end } + } } } _ => PatternKind::Wild From 97a032ebb4f87bb5a0a7478528d4dfb6d2c63ddd Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 24 May 2018 10:48:23 +0100 Subject: [PATCH 19/47] Simplify bitwise operations --- src/librustc_mir/hair/pattern/_match.rs | 69 +++++++++++++++---------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 1c9cdfd03b390..86f0e95a9032a 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -483,11 +483,11 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ty::TyUint(_) if exhaustive_integer_patterns => { // FIXME(49937): refactor these bit manipulations into interpret. let bits = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) - .unwrap().size.bits() as u32; - let max = (!0u128).wrapping_shr(128 - bits); + .unwrap().size.bits() as u128; + let max = !0u128 >> (128 - bits); value_constructors = true; - vec![ConstantRange(ty::Const::from_bits(cx.tcx, 0u128, pcx.ty), - ty::Const::from_bits(cx.tcx, max as u128, pcx.ty), + vec![ConstantRange(ty::Const::from_bits(cx.tcx, 0, pcx.ty), + ty::Const::from_bits(cx.tcx, max, pcx.ty), RangeEnd::Included)] } _ => { @@ -604,21 +604,21 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( } /// An inclusive interval, used for precise integer exhaustiveness checking. -/// `Interval`s always store a contiguous range of integers. This means that -/// signed values are encoded by offsetting them such that `0` represents the -/// minimum value for the integer, regardless of sign. -/// For example, the range `-128...127` is encoded as `0...255`. +/// `IntRange`s always store a contiguous range. This means that values are +/// encoded such that `0` encodes the minimum value for the integer, +/// regardless of the signedness. +/// For example, the pattern `-128...127i8` is encoded as `0..=255`. /// This makes comparisons and arithmetic on interval endpoints much more -/// straightforward. See `offset_sign` for the conversion technique. -struct Interval<'tcx> { +/// straightforward. See `encode` and `decode` for details. +struct IntRange<'tcx> { pub range: RangeInclusive, pub ty: Ty<'tcx>, } -impl<'tcx> Interval<'tcx> { +impl<'tcx> IntRange<'tcx> { fn from_ctor(tcx: TyCtxt<'_, 'tcx, 'tcx>, ctor: &Constructor<'tcx>) - -> Option> { + -> Option> { match ctor { ConstantRange(lo, hi, end) => { assert_eq!(lo.ty, hi.ty); @@ -627,13 +627,13 @@ impl<'tcx> Interval<'tcx> { if let Some(hi) = hi.assert_bits(ty) { // Perform a shift if the underlying types are signed, // which makes the interval arithmetic simpler. - let (lo, hi) = Self::offset_sign(tcx, ty, lo..=hi, true); + let (lo, hi) = Self::encode(tcx, ty, lo..=hi); // Make sure the interval is well-formed. return if lo > hi || lo == hi && *end == RangeEnd::Excluded { None } else { let offset = (*end == RangeEnd::Excluded) as u128; - Some(Interval { range: lo..=(hi - offset), ty }) + Some(IntRange { range: lo..=(hi - offset), ty }) }; } } @@ -642,8 +642,8 @@ impl<'tcx> Interval<'tcx> { ConstantValue(val) => { let ty = val.ty; if let Some(val) = val.assert_bits(ty) { - let (lo, hi) = Self::offset_sign(tcx, ty, val..=val, true); - Some(Interval { range: lo..=hi, ty }) + let (lo, hi) = Self::encode(tcx, ty, val..=val); + Some(IntRange { range: lo..=hi, ty }) } else { None } @@ -654,11 +654,11 @@ impl<'tcx> Interval<'tcx> { } } - fn offset_sign(tcx: TyCtxt<'_, 'tcx, 'tcx>, - ty: Ty<'tcx>, - range: RangeInclusive, - encode: bool) - -> (u128, u128) { + fn convert(tcx: TyCtxt<'_, 'tcx, 'tcx>, + ty: Ty<'tcx>, + range: RangeInclusive, + encode: bool) + -> (u128, u128) { // We ensure that all integer values are contiguous: that is, that their // minimum value is represented by 0, so that comparisons and increments/ // decrements on interval endpoints work consistently whether the endpoints @@ -670,13 +670,14 @@ impl<'tcx> Interval<'tcx> { let bits = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) .unwrap().size.bits() as u128; let min = 1u128 << (bits - 1); - let shift = 1u128.overflowing_shl(bits as u32); - let mask = shift.0.wrapping_sub(1 + (shift.1 as u128)); + let mask = !0u128 >> (128 - bits); if encode { let offset = |x: u128| x.wrapping_sub(min) & mask; (offset(lo), offset(hi)) } else { let offset = |x: u128| { + // FIXME: this shouldn't be necessary once `print_miri_value` + // sign-extends `TyInt`. interpret::sign_extend(tcx, x.wrapping_add(min) & mask, ty) .expect("layout error for TyInt") }; @@ -686,10 +687,24 @@ impl<'tcx> Interval<'tcx> { ty::TyUint(_) | ty::TyChar => { (lo, hi) } - _ => bug!("`Interval` should only contain integer types") + _ => bug!("`IntRange` should only contain integer types") } } + fn encode(tcx: TyCtxt<'_, 'tcx, 'tcx>, + ty: Ty<'tcx>, + range: RangeInclusive) + -> (u128, u128) { + Self::convert(tcx, ty, range, true) + } + + fn decode(tcx: TyCtxt<'_, 'tcx, 'tcx>, + ty: Ty<'tcx>, + range: RangeInclusive) + -> (u128, u128) { + Self::convert(tcx, ty, range, false) + } + fn into_inner(self) -> (u128, u128) { self.range.into_inner() } @@ -702,10 +717,10 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, pat_ctor: &Constructor<'tcx>, ranges: Vec>) -> Vec> { - if let Some(pat_interval) = Interval::from_ctor(cx.tcx, pat_ctor) { + if let Some(pat_interval) = IntRange::from_ctor(cx.tcx, pat_ctor) { let mut remaining_ranges = vec![]; let mut ranges: Vec<_> = ranges.into_iter().filter_map(|r| { - Interval::from_ctor(cx.tcx, &r).map(|i| i.into_inner()) + IntRange::from_ctor(cx.tcx, &r).map(|i| i.into_inner()) }).collect(); let ty = pat_interval.ty; let (pat_interval_lo, pat_interval_hi) = pat_interval.into_inner(); @@ -729,7 +744,7 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } // Convert the remaining ranges from pairs to inclusive `ConstantRange`s. remaining_ranges.into_iter().map(|r| { - let (lo, hi) = Interval::offset_sign(cx.tcx, ty, r, false); + let (lo, hi) = IntRange::decode(cx.tcx, ty, r); ConstantRange(ty::Const::from_bits(cx.tcx, lo, ty), ty::Const::from_bits(cx.tcx, hi, ty), RangeEnd::Included) From be12b242ce08ea7f83ab51b1d0027b9db9f2d737 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 24 May 2018 12:13:28 +0100 Subject: [PATCH 20/47] Fix print_miri_value for signed integers --- src/librustc/mir/mod.rs | 3 ++- src/librustc_mir/hair/pattern/_match.rs | 9 ++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 8ceff303774b5..2013766aa39ce 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -35,6 +35,7 @@ use std::slice; use std::vec::IntoIter; use std::{iter, mem, option, u32}; use syntax::ast::{self, Name}; +use syntax::attr::SignedInt; use syntax::symbol::InternedString; use syntax_pos::{Span, DUMMY_SP}; use ty::fold::{TypeFoldable, TypeFolder, TypeVisitor}; @@ -2228,7 +2229,7 @@ pub fn fmt_const_val(fmt: &mut W, const_val: &ty::Const) -> fmt::Resul } } -pub fn print_miri_value(value: Value, ty: Ty, f: &mut W) -> fmt::Result { +pub fn print_miri_value<'tcx, W: Write>(value: Value, ty: Ty<'tcx>, f: &mut W) -> fmt::Result { use ty::TypeVariants::*; // print some primitives if let Value::Scalar(ScalarMaybeUndef::Scalar(Scalar::Bits { bits, .. })) = value { diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 86f0e95a9032a..c666469f3574d 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -668,19 +668,14 @@ impl<'tcx> IntRange<'tcx> { ty::TyInt(_) => { // FIXME(49937): refactor these bit manipulations into interpret. let bits = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) - .unwrap().size.bits() as u128; + .unwrap().size.bits() as u128; let min = 1u128 << (bits - 1); let mask = !0u128 >> (128 - bits); if encode { let offset = |x: u128| x.wrapping_sub(min) & mask; (offset(lo), offset(hi)) } else { - let offset = |x: u128| { - // FIXME: this shouldn't be necessary once `print_miri_value` - // sign-extends `TyInt`. - interpret::sign_extend(tcx, x.wrapping_add(min) & mask, ty) - .expect("layout error for TyInt") - }; + let offset = |x: u128| x.wrapping_add(min) & mask; (offset(lo), offset(hi)) } } From 72cc4bd33b0d1b975173f4032581040fa354b724 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 24 May 2018 12:27:30 +0100 Subject: [PATCH 21/47] Inline encode and decode methods --- src/librustc_mir/hair/pattern/_match.rs | 63 +++++++++++-------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index c666469f3574d..e498b5582bf05 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -627,7 +627,7 @@ impl<'tcx> IntRange<'tcx> { if let Some(hi) = hi.assert_bits(ty) { // Perform a shift if the underlying types are signed, // which makes the interval arithmetic simpler. - let (lo, hi) = Self::encode(tcx, ty, lo..=hi); + let (lo, hi) = Self::encode(tcx, ty, (lo, hi)); // Make sure the interval is well-formed. return if lo > hi || lo == hi && *end == RangeEnd::Excluded { None @@ -642,7 +642,7 @@ impl<'tcx> IntRange<'tcx> { ConstantValue(val) => { let ty = val.ty; if let Some(val) = val.assert_bits(ty) { - let (lo, hi) = Self::encode(tcx, ty, val..=val); + let (lo, hi) = Self::encode(tcx, ty, (val, val)); Some(IntRange { range: lo..=hi, ty }) } else { None @@ -654,16 +654,10 @@ impl<'tcx> IntRange<'tcx> { } } - fn convert(tcx: TyCtxt<'_, 'tcx, 'tcx>, - ty: Ty<'tcx>, - range: RangeInclusive, - encode: bool) - -> (u128, u128) { - // We ensure that all integer values are contiguous: that is, that their - // minimum value is represented by 0, so that comparisons and increments/ - // decrements on interval endpoints work consistently whether the endpoints - // are signed or unsigned. - let (lo, hi) = range.into_inner(); + fn encode(tcx: TyCtxt<'_, 'tcx, 'tcx>, + ty: Ty<'tcx>, + (lo, hi): (u128, u128)) + -> (u128, u128) { match ty.sty { ty::TyInt(_) => { // FIXME(49937): refactor these bit manipulations into interpret. @@ -671,33 +665,33 @@ impl<'tcx> IntRange<'tcx> { .unwrap().size.bits() as u128; let min = 1u128 << (bits - 1); let mask = !0u128 >> (128 - bits); - if encode { - let offset = |x: u128| x.wrapping_sub(min) & mask; - (offset(lo), offset(hi)) - } else { - let offset = |x: u128| x.wrapping_add(min) & mask; - (offset(lo), offset(hi)) - } + let offset = |x: u128| x.wrapping_sub(min) & mask; + (offset(lo), offset(hi)) } - ty::TyUint(_) | ty::TyChar => { - (lo, hi) - } - _ => bug!("`IntRange` should only contain integer types") + _ => (lo, hi) } } - fn encode(tcx: TyCtxt<'_, 'tcx, 'tcx>, - ty: Ty<'tcx>, - range: RangeInclusive) - -> (u128, u128) { - Self::convert(tcx, ty, range, true) - } - fn decode(tcx: TyCtxt<'_, 'tcx, 'tcx>, ty: Ty<'tcx>, range: RangeInclusive) - -> (u128, u128) { - Self::convert(tcx, ty, range, false) + -> Constructor<'tcx> { + let (lo, hi) = range.into_inner(); + let (lo, hi) = match ty.sty { + ty::TyInt(_) => { + // FIXME(49937): refactor these bit manipulations into interpret. + let bits = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) + .unwrap().size.bits() as u128; + let min = 1u128 << (bits - 1); + let mask = !0u128 >> (128 - bits); + let offset = |x: u128| x.wrapping_add(min) & mask; + (offset(lo), offset(hi)) + } + _ => (lo, hi) + }; + ConstantRange(ty::Const::from_bits(tcx, lo, ty), + ty::Const::from_bits(tcx, hi, ty), + RangeEnd::Included) } fn into_inner(self) -> (u128, u128) { @@ -739,10 +733,7 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } // Convert the remaining ranges from pairs to inclusive `ConstantRange`s. remaining_ranges.into_iter().map(|r| { - let (lo, hi) = IntRange::decode(cx.tcx, ty, r); - ConstantRange(ty::Const::from_bits(cx.tcx, lo, ty), - ty::Const::from_bits(cx.tcx, hi, ty), - RangeEnd::Included) + IntRange::decode(cx.tcx, ty, r) }).collect() } else { ranges From 1aa749469bfc57ae023be6fbb1141b806bcb3d62 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 24 May 2018 13:30:21 +0100 Subject: [PATCH 22/47] Introduce signed_bias method The epitome of simplicity! --- src/librustc_mir/hair/pattern/_match.rs | 66 ++++++++----------------- 1 file changed, 21 insertions(+), 45 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index e498b5582bf05..0d56f345d8eaf 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -21,11 +21,13 @@ use super::{PatternFoldable, PatternFolder, compare_const_vals}; use rustc::hir::def_id::DefId; use rustc::hir::RangeEnd; use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; +use rustc::ty::layout::{Integer, IntegerExt}; use rustc::mir::Field; use rustc::mir::interpret::ConstValue; use rustc::util::common::ErrorReported; +use syntax::attr::{SignedInt, UnsignedInt}; use syntax_pos::{Span, DUMMY_SP}; use arena::TypedArena; @@ -469,10 +471,9 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ConstantRange(endpoint('\u{E000}'), endpoint('\u{10FFFF}'), RangeEnd::Included), ] } - ty::TyInt(_) if exhaustive_integer_patterns => { + ty::TyInt(ity) if exhaustive_integer_patterns => { // FIXME(49937): refactor these bit manipulations into interpret. - let bits = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) - .unwrap().size.bits() as u128; + let bits = Integer::from_attr(cx.tcx, SignedInt(ity)).size().bits() as u128; let min = 1u128 << (bits - 1); let max = (1u128 << (bits - 1)) - 1; value_constructors = true; @@ -480,10 +481,9 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ty::Const::from_bits(cx.tcx, max as u128, pcx.ty), RangeEnd::Included)] } - ty::TyUint(_) if exhaustive_integer_patterns => { + ty::TyUint(uty) if exhaustive_integer_patterns => { // FIXME(49937): refactor these bit manipulations into interpret. - let bits = cx.tcx.layout_of(ty::ParamEnv::reveal_all().and(pcx.ty)) - .unwrap().size.bits() as u128; + let bits = Integer::from_attr(cx.tcx, UnsignedInt(uty)).size().bits() as u128; let max = !0u128 >> (128 - bits); value_constructors = true; vec![ConstantRange(ty::Const::from_bits(cx.tcx, 0, pcx.ty), @@ -627,7 +627,8 @@ impl<'tcx> IntRange<'tcx> { if let Some(hi) = hi.assert_bits(ty) { // Perform a shift if the underlying types are signed, // which makes the interval arithmetic simpler. - let (lo, hi) = Self::encode(tcx, ty, (lo, hi)); + let bias = IntRange::signed_bias(tcx, ty); + let (lo, hi) = (lo ^ bias, hi ^ bias); // Make sure the interval is well-formed. return if lo > hi || lo == hi && *end == RangeEnd::Excluded { None @@ -642,8 +643,9 @@ impl<'tcx> IntRange<'tcx> { ConstantValue(val) => { let ty = val.ty; if let Some(val) = val.assert_bits(ty) { - let (lo, hi) = Self::encode(tcx, ty, (val, val)); - Some(IntRange { range: lo..=hi, ty }) + let bias = IntRange::signed_bias(tcx, ty); + let val = val ^ bias; + Some(IntRange { range: val..=val, ty }) } else { None } @@ -654,46 +656,16 @@ impl<'tcx> IntRange<'tcx> { } } - fn encode(tcx: TyCtxt<'_, 'tcx, 'tcx>, - ty: Ty<'tcx>, - (lo, hi): (u128, u128)) - -> (u128, u128) { + fn signed_bias(tcx: TyCtxt<'_, 'tcx, 'tcx>, ty: Ty<'tcx>) -> u128 { match ty.sty { - ty::TyInt(_) => { - // FIXME(49937): refactor these bit manipulations into interpret. - let bits = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) - .unwrap().size.bits() as u128; - let min = 1u128 << (bits - 1); - let mask = !0u128 >> (128 - bits); - let offset = |x: u128| x.wrapping_sub(min) & mask; - (offset(lo), offset(hi)) + ty::TyInt(ity) => { + let bits = Integer::from_attr(tcx, SignedInt(ity)).size().bits() as u128; + 1u128 << (bits - 1) } - _ => (lo, hi) + _ => 0 } } - fn decode(tcx: TyCtxt<'_, 'tcx, 'tcx>, - ty: Ty<'tcx>, - range: RangeInclusive) - -> Constructor<'tcx> { - let (lo, hi) = range.into_inner(); - let (lo, hi) = match ty.sty { - ty::TyInt(_) => { - // FIXME(49937): refactor these bit manipulations into interpret. - let bits = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)) - .unwrap().size.bits() as u128; - let min = 1u128 << (bits - 1); - let mask = !0u128 >> (128 - bits); - let offset = |x: u128| x.wrapping_add(min) & mask; - (offset(lo), offset(hi)) - } - _ => (lo, hi) - }; - ConstantRange(ty::Const::from_bits(tcx, lo, ty), - ty::Const::from_bits(tcx, hi, ty), - RangeEnd::Included) - } - fn into_inner(self) -> (u128, u128) { self.range.into_inner() } @@ -733,7 +705,11 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } // Convert the remaining ranges from pairs to inclusive `ConstantRange`s. remaining_ranges.into_iter().map(|r| { - IntRange::decode(cx.tcx, ty, r) + let (lo, hi) = r.into_inner(); + let bias = IntRange::signed_bias(cx.tcx, ty); + ConstantRange(ty::Const::from_bits(cx.tcx, lo ^ bias, ty), + ty::Const::from_bits(cx.tcx, hi ^ bias, ty), + RangeEnd::Included) }).collect() } else { ranges From 732d63848302156d475beb7159e73c14c0570e3a Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 25 May 2018 09:47:37 +0100 Subject: [PATCH 23/47] Replace ... with ..= in suggestions As ... is "(silently) deprecated". Presumably this means we should be giving correct, up-to-date suggestions, though. --- src/librustc_mir/hair/pattern/mod.rs | 2 +- src/test/ui/exhaustive_integer_patterns.stderr | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc_mir/hair/pattern/mod.rs b/src/librustc_mir/hair/pattern/mod.rs index 6e56db69a823f..9bf7aa24ae63b 100644 --- a/src/librustc_mir/hair/pattern/mod.rs +++ b/src/librustc_mir/hair/pattern/mod.rs @@ -233,7 +233,7 @@ impl<'tcx> fmt::Display for Pattern<'tcx> { PatternKind::Range { lo, hi, end } => { fmt_const_val(f, lo)?; match end { - RangeEnd::Included => write!(f, "...")?, + RangeEnd::Included => write!(f, "..=")?, RangeEnd::Excluded => write!(f, "..")?, } fmt_const_val(f, hi) diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr index 3d308a9e5d1e6..0f7ab8688c678 100644 --- a/src/test/ui/exhaustive_integer_patterns.stderr +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -10,17 +10,17 @@ note: lint level defined here LL | #![deny(unreachable_patterns)] | ^^^^^^^^^^^^^^^^^^^^ -error[E0004]: non-exhaustive patterns: `128u8...255u8` not covered +error[E0004]: non-exhaustive patterns: `128u8..=255u8` not covered --> $DIR/exhaustive_integer_patterns.rs:37:11 | LL | match x { //~ ERROR non-exhaustive patterns - | ^ pattern `128u8...255u8` not covered + | ^ pattern `128u8..=255u8` not covered -error[E0004]: non-exhaustive patterns: `11u8...19u8`, `31u8...34u8`, `36u8...69u8` and 1 more not covered +error[E0004]: non-exhaustive patterns: `11u8..=19u8`, `31u8..=34u8`, `36u8..=69u8` and 1 more not covered --> $DIR/exhaustive_integer_patterns.rs:42:11 | LL | match x { //~ ERROR non-exhaustive patterns - | ^ patterns `11u8...19u8`, `31u8...34u8`, `36u8...69u8` and 1 more not covered + | ^ patterns `11u8..=19u8`, `31u8..=34u8`, `36u8..=69u8` and 1 more not covered error: unreachable pattern --> $DIR/exhaustive_integer_patterns.rs:53:9 @@ -28,11 +28,11 @@ error: unreachable pattern LL | -2..=20 => {} //~ ERROR unreachable pattern | ^^^^^^^ -error[E0004]: non-exhaustive patterns: `-128i8...-8i8`, `-6i8`, `121i8...124i8` and 1 more not covered +error[E0004]: non-exhaustive patterns: `-128i8..=-8i8`, `-6i8`, `121i8..=124i8` and 1 more not covered --> $DIR/exhaustive_integer_patterns.rs:50:11 | LL | match x { //~ ERROR non-exhaustive patterns - | ^ patterns `-128i8...-8i8`, `-6i8`, `121i8...124i8` and 1 more not covered + | ^ patterns `-128i8..=-8i8`, `-6i8`, `121i8..=124i8` and 1 more not covered error[E0004]: non-exhaustive patterns: `-128i8` not covered --> $DIR/exhaustive_integer_patterns.rs:99:11 From 6c21a0322c55c2da7f2b1479cb631b8596901f1c Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 26 May 2018 00:54:52 +0100 Subject: [PATCH 24/47] Refactor after miri api changes --- src/librustc/mir/mod.rs | 1 - src/librustc_mir/hair/pattern/_match.rs | 27 +++++++++++++++---------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 2013766aa39ce..8b5b41fc777ea 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -35,7 +35,6 @@ use std::slice; use std::vec::IntoIter; use std::{iter, mem, option, u32}; use syntax::ast::{self, Name}; -use syntax::attr::SignedInt; use syntax::symbol::InternedString; use syntax_pos::{Span, DUMMY_SP}; use ty::fold::{TypeFoldable, TypeFolder, TypeVisitor}; diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 0d56f345d8eaf..9cd59a42402ba 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -461,10 +461,11 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, .collect() } ty::TyChar if exhaustive_integer_patterns => { + value_constructors = true; let endpoint = |c: char| { - ty::Const::from_bits(cx.tcx, c as u128, cx.tcx.types.char) + let ty = ty::ParamEnv::empty().and(cx.tcx.types.char); + ty::Const::from_bits(cx.tcx, c as u128, ty) }; - value_constructors = true; vec![ // The valid Unicode Scalar Value ranges. ConstantRange(endpoint('\u{0000}'), endpoint('\u{D7FF}'), RangeEnd::Included), @@ -472,22 +473,24 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ] } ty::TyInt(ity) if exhaustive_integer_patterns => { + value_constructors = true; // FIXME(49937): refactor these bit manipulations into interpret. let bits = Integer::from_attr(cx.tcx, SignedInt(ity)).size().bits() as u128; let min = 1u128 << (bits - 1); let max = (1u128 << (bits - 1)) - 1; - value_constructors = true; - vec![ConstantRange(ty::Const::from_bits(cx.tcx, min as u128, pcx.ty), - ty::Const::from_bits(cx.tcx, max as u128, pcx.ty), + let ty = ty::ParamEnv::empty().and(pcx.ty); + vec![ConstantRange(ty::Const::from_bits(cx.tcx, min as u128, ty), + ty::Const::from_bits(cx.tcx, max as u128, ty), RangeEnd::Included)] } ty::TyUint(uty) if exhaustive_integer_patterns => { + value_constructors = true; // FIXME(49937): refactor these bit manipulations into interpret. let bits = Integer::from_attr(cx.tcx, UnsignedInt(uty)).size().bits() as u128; let max = !0u128 >> (128 - bits); - value_constructors = true; - vec![ConstantRange(ty::Const::from_bits(cx.tcx, 0, pcx.ty), - ty::Const::from_bits(cx.tcx, max, pcx.ty), + let ty = ty::ParamEnv::empty().and(pcx.ty); + vec![ConstantRange(ty::Const::from_bits(cx.tcx, 0, ty), + ty::Const::from_bits(cx.tcx, max, ty), RangeEnd::Included)] } _ => { @@ -623,8 +626,9 @@ impl<'tcx> IntRange<'tcx> { ConstantRange(lo, hi, end) => { assert_eq!(lo.ty, hi.ty); let ty = lo.ty; - if let Some(lo) = lo.assert_bits(ty) { - if let Some(hi) = hi.assert_bits(ty) { + let env_ty = ty::ParamEnv::empty().and(ty); + if let Some(lo) = lo.assert_bits(tcx, env_ty) { + if let Some(hi) = hi.assert_bits(tcx, env_ty) { // Perform a shift if the underlying types are signed, // which makes the interval arithmetic simpler. let bias = IntRange::signed_bias(tcx, ty); @@ -642,7 +646,7 @@ impl<'tcx> IntRange<'tcx> { } ConstantValue(val) => { let ty = val.ty; - if let Some(val) = val.assert_bits(ty) { + if let Some(val) = val.assert_bits(tcx, ty::ParamEnv::empty().and(ty)) { let bias = IntRange::signed_bias(tcx, ty); let val = val ^ bias; Some(IntRange { range: val..=val, ty }) @@ -707,6 +711,7 @@ fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, remaining_ranges.into_iter().map(|r| { let (lo, hi) = r.into_inner(); let bias = IntRange::signed_bias(cx.tcx, ty); + let ty = ty::ParamEnv::empty().and(ty); ConstantRange(ty::Const::from_bits(cx.tcx, lo ^ bias, ty), ty::Const::from_bits(cx.tcx, hi ^ bias, ty), RangeEnd::Included) From d27c21c016ed8d9a4efaef0a0e86f771c82bf5d8 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 28 May 2018 20:53:20 +0100 Subject: [PATCH 25/47] Refactor for less allocation --- src/librustc_mir/hair/pattern/_match.rs | 119 ++++++++++++------------ src/librustc_mir/hair/pattern/mod.rs | 17 ++-- 2 files changed, 69 insertions(+), 67 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 9cd59a42402ba..4cd70a0f57d31 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -612,7 +612,7 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( /// regardless of the signedness. /// For example, the pattern `-128...127i8` is encoded as `0..=255`. /// This makes comparisons and arithmetic on interval endpoints much more -/// straightforward. See `encode` and `decode` for details. +/// straightforward. See `signed_bias` for details. struct IntRange<'tcx> { pub range: RangeInclusive, pub ty: Ty<'tcx>, @@ -660,6 +660,8 @@ impl<'tcx> IntRange<'tcx> { } } + // The return value of `signed_bias` should be + // XORed with an endpoint to encode/decode it. fn signed_bias(tcx: TyCtxt<'_, 'tcx, 'tcx>, ty: Ty<'tcx>) -> u128 { match ty.sty { ty::TyInt(ity) => { @@ -670,54 +672,52 @@ impl<'tcx> IntRange<'tcx> { } } - fn into_inner(self) -> (u128, u128) { - self.range.into_inner() - } -} - -/// Given a pattern in a `match` and a collection of ranges corresponding to the -/// domain of values of a type (say, an integer), return a new collection of ranges -/// corresponding to those ranges minus the ranges covered by the pattern. -fn ranges_subtract_pattern<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, - pat_ctor: &Constructor<'tcx>, - ranges: Vec>) - -> Vec> { - if let Some(pat_interval) = IntRange::from_ctor(cx.tcx, pat_ctor) { + /// Given an `IntRange` corresponding to a pattern in a `match` and a collection of + /// ranges corresponding to the domain of values of a type (say, an integer), return + /// a new collection of ranges corresponding to the original ranges minus the ranges + /// covered by the `IntRange`. + fn subtract_from(self, + tcx: TyCtxt<'_, 'tcx, 'tcx>, + ranges: Vec>) + -> Vec> { + let ranges = ranges.into_iter().filter_map(|r| { + IntRange::from_ctor(tcx, &r).map(|i| i.range) + }); + // Convert a `RangeInclusive` to a `ConstantValue` or inclusive `ConstantRange`. + let bias = IntRange::signed_bias(tcx, self.ty); + let ty = ty::ParamEnv::empty().and(self.ty); + let range_to_constant = |r: RangeInclusive| { + let (lo, hi) = r.into_inner(); + if lo == hi { + ConstantValue(ty::Const::from_bits(tcx, lo ^ bias, ty)) + } else { + ConstantRange(ty::Const::from_bits(tcx, lo ^ bias, ty), + ty::Const::from_bits(tcx, hi ^ bias, ty), + RangeEnd::Included) + } + }; let mut remaining_ranges = vec![]; - let mut ranges: Vec<_> = ranges.into_iter().filter_map(|r| { - IntRange::from_ctor(cx.tcx, &r).map(|i| i.into_inner()) - }).collect(); - let ty = pat_interval.ty; - let (pat_interval_lo, pat_interval_hi) = pat_interval.into_inner(); - for (subrange_lo, subrange_hi) in ranges { - if pat_interval_lo > subrange_hi || subrange_lo > pat_interval_hi { + let (lo, hi) = self.range.into_inner(); + for subrange in ranges { + let (subrange_lo, subrange_hi) = subrange.into_inner(); + 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(subrange_lo..=subrange_hi); + remaining_ranges.push(range_to_constant(subrange_lo..=subrange_hi)); } else { - if pat_interval_lo > subrange_lo { + if lo > subrange_lo { // The pattern intersects an upper section of the // subrange, so a lower section will remain. - remaining_ranges.push(subrange_lo..=(pat_interval_lo - 1)); + remaining_ranges.push(range_to_constant(subrange_lo..=(lo - 1))); } - if pat_interval_hi < subrange_hi { + if hi < subrange_hi { // The pattern intersects a lower section of the // subrange, so an upper section will remain. - remaining_ranges.push((pat_interval_hi + 1)..=subrange_hi); + remaining_ranges.push(range_to_constant((hi + 1)..=subrange_hi)); } } } - // Convert the remaining ranges from pairs to inclusive `ConstantRange`s. - remaining_ranges.into_iter().map(|r| { - let (lo, hi) = r.into_inner(); - let bias = IntRange::signed_bias(cx.tcx, ty); - let ty = ty::ParamEnv::empty().and(ty); - ConstantRange(ty::Const::from_bits(cx.tcx, lo ^ bias, ty), - ty::Const::from_bits(cx.tcx, hi ^ bias, ty), - RangeEnd::Included) - }).collect() - } else { - ranges + remaining_ranges } } @@ -818,8 +818,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // `missing_ctors` are those that should have appeared // as patterns in the `match` expression, but did not. let mut missing_ctors = vec![]; - 'req: for req_ctor in &all_ctors { - let mut sub_ctors = vec![req_ctor.clone()]; + for req_ctor in &all_ctors { // The only constructor patterns for which it is valid to // treat the values as constructors are ranges (see // `all_constructors` for details). @@ -827,29 +826,33 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ConstantRange(..) => true, _ => false, }; - for used_ctor in &used_ctors { - if consider_value_constructors { - sub_ctors = ranges_subtract_pattern(cx, used_ctor, sub_ctors); + if consider_value_constructors { + let mut refined_ctors = vec![req_ctor.clone()]; + for used_ctor in &used_ctors { + // Refine the required constructors for the type by subtracting + // the range defined by the current constructor pattern. + refined_ctors = match IntRange::from_ctor(cx.tcx, used_ctor) { + Some(interval) => interval.subtract_from(cx.tcx, refined_ctors), + None => refined_ctors, + }; // If the constructor patterns that have been considered so far // already cover the entire range of values, then we the // constructor is not missing, and we can move on to the next one. - if sub_ctors.is_empty() { - continue 'req; - } - } else { - // If the pattern for the required constructor - // appears in the `match`, then it is not missing, - // and we can move on to the next one. - if used_ctor == req_ctor { - continue 'req; + if refined_ctors.is_empty() { + break; } } + // If a constructor has not been matched, then it is missing. + // We add `refined_ctors` instead of `req_ctor`, because then we can + // provide more detailed error information about precisely which + // ranges have been omitted. + missing_ctors.extend(refined_ctors); + } else { + // A constructor is missing if it never appears in a `match` arm. + if !used_ctors.iter().any(|used_ctor| used_ctor == req_ctor) { + missing_ctors.push(req_ctor.clone()); + } } - // If a constructor has not been matched, then it is missing. - // We add `sub_ctors` instead of `req_ctor`, because then we can - // provide more detailed error information about precisely which - // ranges have been omitted. - missing_ctors.extend(sub_ctors); } // `missing_ctors` is the set of constructors from the same type as the @@ -968,11 +971,11 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, match ctor { // A constant range of length 1 is simply // a constant value. - ConstantRange(lo, hi, _) if lo == hi => { + ConstantValue(value) => { Witness(vec![Pattern { ty: pcx.ty, span: DUMMY_SP, - kind: box PatternKind::Constant { value: lo }, + kind: box PatternKind::Constant { value }, }]) } // We always report missing intervals diff --git a/src/librustc_mir/hair/pattern/mod.rs b/src/librustc_mir/hair/pattern/mod.rs index 9bf7aa24ae63b..7ec9373130419 100644 --- a/src/librustc_mir/hair/pattern/mod.rs +++ b/src/librustc_mir/hair/pattern/mod.rs @@ -368,9 +368,14 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { "lower range bound must be less than upper", ); PatternKind::Wild - }, - (RangeEnd::Included, None) | - (RangeEnd::Included, Some(Ordering::Greater)) => { + } + (RangeEnd::Included, Some(Ordering::Equal)) => { + PatternKind::Constant { value: lo } + } + (RangeEnd::Included, Some(Ordering::Less)) => { + PatternKind::Range { lo, hi, end } + } + (RangeEnd::Included, _) => { let mut err = struct_span_err!( self.tcx.sess, lo_expr.span, @@ -390,12 +395,6 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { } err.emit(); PatternKind::Wild - }, - (RangeEnd::Included, Some(Ordering::Equal)) => { - PatternKind::Constant { value: lo } - } - (RangeEnd::Included, Some(Ordering::Less)) => { - PatternKind::Range { lo, hi, end } } } } From 07064de9a788d02b28f8e3f32d81334cd8ef2dce Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 29 May 2018 12:47:39 +0100 Subject: [PATCH 26/47] No longer return value_constructors for all_constructors --- src/librustc_mir/hair/pattern/_match.rs | 33 +++++++++++-------------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 4cd70a0f57d31..5d990dfa8dfe3 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -426,11 +426,10 @@ impl<'tcx> Witness<'tcx> { /// Option we do not include Some(_) in the returned list of constructors. fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, pcx: PatternContext<'tcx>) - -> (Vec>, bool) + -> Vec> { debug!("all_constructors({:?})", pcx.ty); let exhaustive_integer_patterns = cx.tcx.features().exhaustive_integer_patterns; - let mut value_constructors = false; let ctors = match pcx.ty.sty { ty::TyBool => { [true, false].iter().map(|&b| { @@ -461,7 +460,6 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, .collect() } ty::TyChar if exhaustive_integer_patterns => { - value_constructors = true; let endpoint = |c: char| { let ty = ty::ParamEnv::empty().and(cx.tcx.types.char); ty::Const::from_bits(cx.tcx, c as u128, ty) @@ -473,7 +471,6 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ] } ty::TyInt(ity) if exhaustive_integer_patterns => { - value_constructors = true; // FIXME(49937): refactor these bit manipulations into interpret. let bits = Integer::from_attr(cx.tcx, SignedInt(ity)).size().bits() as u128; let min = 1u128 << (bits - 1); @@ -484,7 +481,6 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, RangeEnd::Included)] } ty::TyUint(uty) if exhaustive_integer_patterns => { - value_constructors = true; // FIXME(49937): refactor these bit manipulations into interpret. let bits = Integer::from_attr(cx.tcx, UnsignedInt(uty)).size().bits() as u128; let max = !0u128 >> (128 - bits); @@ -501,7 +497,7 @@ fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } } }; - (ctors, value_constructors) + ctors } fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( @@ -810,22 +806,23 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, debug!("used_ctors = {:#?}", used_ctors); // `all_ctors` are all the constructors for the given type, which // should all be represented (or caught with the wild pattern `_`). - // `value_constructors` is true if we may exhaustively consider all - // the possible values (e.g. integers) of a type as its constructors. - let (all_ctors, value_constructors) = all_constructors(cx, pcx); + let all_ctors = all_constructors(cx, pcx); debug!("all_ctors = {:#?}", all_ctors); + // The only constructor patterns for which it is valid to + // treat the values as constructors are ranges (see + // `all_constructors` for details). + let exhaustive_integer_patterns = cx.tcx.features().exhaustive_integer_patterns; + let consider_value_constructors = exhaustive_integer_patterns + && all_ctors.iter().all(|ctor| match ctor { + ConstantRange(..) => true, + _ => false, + }); + // `missing_ctors` are those that should have appeared // as patterns in the `match` expression, but did not. let mut missing_ctors = vec![]; for req_ctor in &all_ctors { - // The only constructor patterns for which it is valid to - // treat the values as constructors are ranges (see - // `all_constructors` for details). - let consider_value_constructors = value_constructors && match req_ctor { - ConstantRange(..) => true, - _ => false, - }; if consider_value_constructors { let mut refined_ctors = vec![req_ctor.clone()]; for used_ctor in &used_ctors { @@ -886,7 +883,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let is_non_exhaustive = is_privately_empty || is_declared_nonexhaustive; if missing_ctors.is_empty() && !is_non_exhaustive { - if value_constructors { + if consider_value_constructors { // If we've successfully matched every value // of the type, then we're done. NotUseful @@ -962,7 +959,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, witness }).collect() } else { - if value_constructors { + if consider_value_constructors { // If we've been trying to exhaustively match // over the domain of values for a type, // then we can provide better diagnostics From 25ba9118ff3d3f6b8d05efb7995c4d53a55841b8 Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 2 Jun 2018 10:53:47 +0100 Subject: [PATCH 27/47] Add guarded arms to tests --- src/test/ui/exhaustive_integer_patterns.rs | 12 ++++++++++++ src/test/ui/exhaustive_integer_patterns.stderr | 8 +++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs index e8f2affbec358..294481389f713 100644 --- a/src/test/ui/exhaustive_integer_patterns.rs +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -120,4 +120,16 @@ fn main() { match 0i128 { i128::MIN ..= i128::MAX => {} // ok } + + // Make sure that guards don't factor into the exhaustiveness checks. + match 0u8 { //~ ERROR non-exhaustive patterns + 0 .. 128 => {} + 128 ..= 255 if true => {} + } + + match 0u8 { + 0 .. 128 => {} + 128 ..= 255 if false => {} + 128 ..= 255 => {} // ok, because previous arm was guarded + } } diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr index 0f7ab8688c678..a2ed9416b50bb 100644 --- a/src/test/ui/exhaustive_integer_patterns.stderr +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -46,6 +46,12 @@ error[E0004]: non-exhaustive patterns: `0i16` not covered LL | match 0i16 { //~ ERROR non-exhaustive patterns | ^^^^ pattern `0i16` not covered -error: aborting due to 7 previous errors +error[E0004]: non-exhaustive patterns: `128u8..=255u8` not covered + --> $DIR/exhaustive_integer_patterns.rs:125:11 + | +LL | match 0u8 { //~ ERROR non-exhaustive patterns + | ^^^ pattern `128u8..=255u8` not covered + +error: aborting due to 8 previous errors For more information about this error, try `rustc --explain E0004`. From af366b0eb86eb7111150b4143167440532e09520 Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 22 Jun 2018 23:52:56 +0100 Subject: [PATCH 28/47] Refactor condition --- src/librustc_mir/hair/pattern/_match.rs | 58 ++++++++++++------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 5d990dfa8dfe3..60a8c89995a00 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -35,6 +35,7 @@ use arena::TypedArena; use std::cmp::{self, Ordering}; use std::fmt; use std::iter::{FromIterator, IntoIterator}; +use std::ops::RangeInclusive; pub fn expand_pattern<'a, 'tcx>(cx: &MatchCheckCtxt<'a, 'tcx>, pat: Pattern<'tcx>) -> &'a Pattern<'tcx> @@ -274,7 +275,7 @@ impl<'tcx> Constructor<'tcx> { } } -#[derive(Clone)] +#[derive(Clone, Debug)] pub enum Usefulness<'tcx> { Useful, UsefulWithWitness(Vec>), @@ -290,7 +291,7 @@ impl<'tcx> Usefulness<'tcx> { } } -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug)] pub enum WitnessPreference { ConstructWitness, LeaveOutWitness @@ -303,7 +304,7 @@ struct PatternContext<'tcx> { } /// A stack of patterns in reverse order of construction -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct Witness<'tcx>(Vec>); impl<'tcx> Witness<'tcx> { @@ -418,10 +419,6 @@ impl<'tcx> Witness<'tcx> { /// but is instead bounded by the maximum fixed length of slice patterns in /// the column of patterns being analyzed. /// -/// This intentionally does not list ConstantValue specializations for -/// non-booleans, because we currently assume that there is always a -/// "non-standard constant" that matches. See issue #12483. -/// /// We make sure to omit constructors that are statically impossible. eg for /// Option we do not include Some(_) in the returned list of constructors. fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, @@ -530,7 +527,7 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( // `[true, ..]` // `[.., false]` // Then any slice of length ≥1 that matches one of these two - // patterns can be be trivially turned to a slice of any + // patterns can be trivially turned to a slice of any // other length ≥1 that matches them and vice-versa - for // but the slice from length 2 `[false, true]` that matches neither // of these patterns can't be turned to a slice from length 1 that @@ -823,33 +820,32 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // as patterns in the `match` expression, but did not. let mut missing_ctors = vec![]; for req_ctor in &all_ctors { - if consider_value_constructors { - let mut refined_ctors = vec![req_ctor.clone()]; - for used_ctor in &used_ctors { - // Refine the required constructors for the type by subtracting - // the range defined by the current constructor pattern. - refined_ctors = match IntRange::from_ctor(cx.tcx, used_ctor) { - Some(interval) => interval.subtract_from(cx.tcx, refined_ctors), - None => refined_ctors, - }; - // If the constructor patterns that have been considered so far - // already cover the entire range of values, then we the - // constructor is not missing, and we can move on to the next one. - if refined_ctors.is_empty() { - break; + let mut refined_ctors = vec![req_ctor.clone()]; + for used_ctor in &used_ctors { + if used_ctor == req_ctor { + // If a constructor appears in a `match` arm, we can + // eliminate it straight away. + refined_ctors = vec![] + } else if exhaustive_integer_patterns { + if let Some(interval) = IntRange::from_ctor(cx.tcx, used_ctor) { + // Refine the required constructors for the type by subtracting + // the range defined by the current constructor pattern. + refined_ctors = interval.subtract_from(cx.tcx, refined_ctors); } } - // If a constructor has not been matched, then it is missing. - // We add `refined_ctors` instead of `req_ctor`, because then we can - // provide more detailed error information about precisely which - // ranges have been omitted. - missing_ctors.extend(refined_ctors); - } else { - // A constructor is missing if it never appears in a `match` arm. - if !used_ctors.iter().any(|used_ctor| used_ctor == req_ctor) { - missing_ctors.push(req_ctor.clone()); + + // If the constructor patterns that have been considered so far + // already cover the entire range of values, then we the + // constructor is not missing, and we can move on to the next one. + if refined_ctors.is_empty() { + break; } } + // If a constructor has not been matched, then it is missing. + // We add `refined_ctors` instead of `req_ctor`, because then we can + // provide more detailed error information about precisely which + // ranges have been omitted. + missing_ctors.extend(refined_ctors); } // `missing_ctors` is the set of constructors from the same type as the From bfc0807b28fe51f71fcf30cd60ddc8a09e4f730d Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 12 Aug 2018 11:43:42 +0100 Subject: [PATCH 29/47] Add some comments --- src/librustc_mir/hair/pattern/_match.rs | 37 ++++++++++++++++++++++++- src/libsyntax/feature_gate.rs | 2 +- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 60a8c89995a00..00f7da4329100 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -303,7 +303,38 @@ struct PatternContext<'tcx> { max_slice_length: u64, } -/// A stack of patterns in reverse order of construction +/// A witness of non-exhaustiveness for error reporting, represented +/// as a list of patterns (in reverse order of construction) with +/// wildcards inside to represent elements that can take any inhabitant +/// of the type as a value. +/// +/// A witness against a list of patterns should have the same types +/// and length as the pattern matched against. Because Rust `match` +/// is always against a single pattern, at the end the witness will +/// have length 1, but in the middle of the algorithm, it can contain +/// multiple patterns. +/// +/// For example, if we are constructing a witness for the match against +/// ``` +/// struct Pair(Option<(u32, u32)>, bool); +/// +/// match (p: Pair) { +/// Pair(None, _) => {} +/// Pair(_, false) => {} +/// } +/// ``` +/// +/// We'll perform the following steps: +/// 1. Start with an empty witness +/// `Witness(vec![])` +/// 2. Push a witness `Some(_)` against the `None` +/// `Witness(vec![Some(_)])` +/// 3. Push a witness `true` against the `false` +/// `Witness(vec![Some(_), true])` +/// 4. Apply the `Pair` constructor to the witnesses +/// `Witness(vec![Pair(Some(_), true)])` +/// +/// The final `Pair(Some(_), true)` is then the resulting witness. #[derive(Clone, Debug)] pub struct Witness<'tcx>(Vec>); @@ -987,6 +1018,10 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } else { pats.into_iter().flat_map(|witness| { missing_ctors.iter().map(move |ctor| { + // Extends the witness with a "wild" version of this + // constructor, that matches everything that can be built with + // it. For example, if `ctor` is a `Constructor::Variant` for + // `Option::Some`, this pushes the witness for `Some(_)`. witness.clone().push_wild_constructor(cx, ctor, pcx.ty) }) }).collect() diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 6109e5ecb61c0..a6908619d73d3 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -488,7 +488,7 @@ declare_features! ( (active, label_break_value, "1.28.0", Some(48594), None), // Integer match exhaustiveness checking - (active, exhaustive_integer_patterns, "1.28.0", Some(50907), None), + (active, exhaustive_integer_patterns, "1.30.0", Some(50907), None), // #[panic_implementation] (active, panic_implementation, "1.28.0", Some(44489), None), From 4aa929cf8b47840671bf2c9a84f70abaead70a85 Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 12 Aug 2018 20:16:25 +0100 Subject: [PATCH 30/47] Move witnesses inside push_wild_constructor --- src/librustc_mir/hair/pattern/_match.rs | 90 +++++++++++-------------- 1 file changed, 41 insertions(+), 49 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 00f7da4329100..065fbeda15360 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -351,15 +351,38 @@ impl<'tcx> Witness<'tcx> { ty: Ty<'tcx>) -> Self { - let sub_pattern_tys = constructor_sub_pattern_tys(cx, ctor, ty); - self.0.extend(sub_pattern_tys.into_iter().map(|ty| { - Pattern { - ty, - span: DUMMY_SP, - kind: box PatternKind::Wild, + // If we've been trying to exhaustively match over the domain of values for a type, + // then we can construct witnesses directly corresponding to the missing ranges of values, + // giving far more precise diagnostics. + // `ConstantValue` and `ConstantRange` only occur in practice when doing exhaustive value + // matching (exhaustive_integer_patterns). + match ctor { + ConstantValue(value) => { + Witness(vec![Pattern { + ty, + span: DUMMY_SP, + kind: box PatternKind::Constant { value }, + }]) + } + ConstantRange(lo, hi, end) => { + Witness(vec![Pattern { + ty, + span: DUMMY_SP, + kind: box PatternKind::Range { lo, hi, end: *end }, + }]) } - })); - self.apply_constructor(cx, ctor, ty) + _ => { + let sub_pattern_tys = constructor_sub_pattern_tys(cx, ctor, ty); + self.0.extend(sub_pattern_tys.into_iter().map(|ty| { + Pattern { + ty, + span: DUMMY_SP, + kind: box PatternKind::Wild, + } + })); + self.apply_constructor(cx, ctor, ty) + } + } } @@ -976,7 +999,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // `used_ctors` is empty. let new_witnesses = if is_non_exhaustive || used_ctors.is_empty() { // All constructors are unused. Add wild patterns - // rather than each individual constructor + // rather than each individual constructor. pats.into_iter().map(|mut witness| { witness.0.push(Pattern { ty: pcx.ty, @@ -986,46 +1009,15 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, witness }).collect() } else { - if consider_value_constructors { - // If we've been trying to exhaustively match - // over the domain of values for a type, - // then we can provide better diagnostics - // regarding which values were missing. - missing_ctors.into_iter().map(|ctor| { - match ctor { - // A constant range of length 1 is simply - // a constant value. - ConstantValue(value) => { - Witness(vec![Pattern { - ty: pcx.ty, - span: DUMMY_SP, - kind: box PatternKind::Constant { value }, - }]) - } - // We always report missing intervals - // in terms of inclusive ranges. - ConstantRange(lo, hi, end) => { - Witness(vec![Pattern { - ty: pcx.ty, - span: DUMMY_SP, - kind: box PatternKind::Range { lo, hi, end }, - }]) - }, - _ => bug!("`ranges_subtract_pattern` should only produce \ - `ConstantRange`s"), - } - }).collect() - } else { - pats.into_iter().flat_map(|witness| { - missing_ctors.iter().map(move |ctor| { - // Extends the witness with a "wild" version of this - // constructor, that matches everything that can be built with - // it. For example, if `ctor` is a `Constructor::Variant` for - // `Option::Some`, this pushes the witness for `Some(_)`. - witness.clone().push_wild_constructor(cx, ctor, pcx.ty) - }) - }).collect() - } + pats.into_iter().flat_map(|witness| { + missing_ctors.iter().map(move |ctor| { + // Extends the witness with a "wild" version of this + // constructor, that matches everything that can be built with + // it. For example, if `ctor` is a `Constructor::Variant` for + // `Option::Some`, this pushes the witness for `Some(_)`. + witness.clone().push_wild_constructor(cx, ctor, pcx.ty) + }) + }).collect() }; UsefulWithWitness(new_witnesses) } From 5959a358e47ac438e924c421e739b10f2c127911 Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 12 Aug 2018 20:39:53 +0100 Subject: [PATCH 31/47] Move logic from push_wild_constructor to apply_constructor --- src/librustc_mir/hair/pattern/_match.rs | 42 ++++++------------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 065fbeda15360..26df066634015 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -351,38 +351,15 @@ impl<'tcx> Witness<'tcx> { ty: Ty<'tcx>) -> Self { - // If we've been trying to exhaustively match over the domain of values for a type, - // then we can construct witnesses directly corresponding to the missing ranges of values, - // giving far more precise diagnostics. - // `ConstantValue` and `ConstantRange` only occur in practice when doing exhaustive value - // matching (exhaustive_integer_patterns). - match ctor { - ConstantValue(value) => { - Witness(vec![Pattern { - ty, - span: DUMMY_SP, - kind: box PatternKind::Constant { value }, - }]) - } - ConstantRange(lo, hi, end) => { - Witness(vec![Pattern { - ty, - span: DUMMY_SP, - kind: box PatternKind::Range { lo, hi, end: *end }, - }]) + let sub_pattern_tys = constructor_sub_pattern_tys(cx, ctor, ty); + self.0.extend(sub_pattern_tys.into_iter().map(|ty| { + Pattern { + ty, + span: DUMMY_SP, + kind: box PatternKind::Wild, } - _ => { - let sub_pattern_tys = constructor_sub_pattern_tys(cx, ctor, ty); - self.0.extend(sub_pattern_tys.into_iter().map(|ty| { - Pattern { - ty, - span: DUMMY_SP, - kind: box PatternKind::Wild, - } - })); - self.apply_constructor(cx, ctor, ty) - } - } + })); + self.apply_constructor(cx, ctor, ty) } @@ -409,7 +386,7 @@ impl<'tcx> Witness<'tcx> { let arity = constructor_arity(cx, ctor, ty); let pat = { let len = self.0.len() as u64; - let mut pats = self.0.drain((len-arity) as usize..).rev(); + let mut pats = self.0.drain((len - arity) as usize..).rev(); match ty.sty { ty::TyAdt(..) | @@ -452,6 +429,7 @@ impl<'tcx> Witness<'tcx> { _ => { match *ctor { ConstantValue(value) => PatternKind::Constant { value }, + ConstantRange(lo, hi, end) => PatternKind::Range { lo, hi, end }, _ => PatternKind::Wild, } } From bfc8ce36f8978994c4bc3df9f210cb2d15363f02 Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 12 Aug 2018 20:47:23 +0100 Subject: [PATCH 32/47] Add a test for integer products --- src/test/ui/exhaustive_integer_patterns.rs | 6 ++++++ src/test/ui/exhaustive_integer_patterns.stderr | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs index 294481389f713..c267c4ef28c08 100644 --- a/src/test/ui/exhaustive_integer_patterns.rs +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -132,4 +132,10 @@ fn main() { 128 ..= 255 if false => {} 128 ..= 255 => {} // ok, because previous arm was guarded } + + // Now things start getting a bit more interesting. Testing products! + match (0u8, Some(())) { //~ ERROR non-exhaustive patterns + (1, _) => {} + (_, None) => {} + } } diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr index a2ed9416b50bb..6fabbebf4875a 100644 --- a/src/test/ui/exhaustive_integer_patterns.stderr +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -52,6 +52,12 @@ error[E0004]: non-exhaustive patterns: `128u8..=255u8` not covered LL | match 0u8 { //~ ERROR non-exhaustive patterns | ^^^ pattern `128u8..=255u8` not covered -error: aborting due to 8 previous errors +error[E0004]: non-exhaustive patterns: `(0u8, Some(_))` and `(2u8..=255u8, Some(_))` not covered + --> $DIR/exhaustive_integer_patterns.rs:137:11 + | +LL | match (0u8, Some(())) { //~ ERROR non-exhaustive patterns + | ^^^^^^^^^^^^^^^ patterns `(0u8, Some(_))` and `(2u8..=255u8, Some(_))` not covered + +error: aborting due to 9 previous errors For more information about this error, try `rustc --explain E0004`. From 99754adbbb423a325b03ce99d9d7f89128842ba9 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 13 Aug 2018 00:56:13 +0100 Subject: [PATCH 33/47] Some reformatting --- src/librustc_mir/hair/pattern/_match.rs | 9 ++++----- src/librustc_mir/hair/pattern/check_match.rs | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 26df066634015..5af9173be0337 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -814,8 +814,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'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), + 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]))) }; @@ -1046,7 +1045,7 @@ fn is_useful_specialized<'p, 'a:'p, 'tcx: 'a>( /// Slice patterns, however, can match slices of different lengths. For instance, /// `[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. +/// Returns `None` in case of a catch-all, which can't be specialized. fn pat_constructors<'tcx>(cx: &mut MatchCheckCtxt, pat: &Pattern<'tcx>, pcx: PatternContext) @@ -1319,13 +1318,13 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( span_bug!(pat.span, "unexpected const-val {:?} with ctor {:?}", value, constructor) } - }, + } _ => { match constructor_covered_by_range( cx.tcx, constructor, value, value, RangeEnd::Included, value.ty, - ) { + ) { Ok(true) => Some(vec![]), Ok(false) => None, Err(ErrorReported) => None, diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index 35f9dcee99f82..3a1e29dfe97b0 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -272,7 +272,7 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> { self.tables); let pattern = patcx.lower_pattern(pat); let pattern_ty = pattern.ty; - let pats : Matrix = vec![vec![ + let pats: Matrix = vec![vec![ expand_pattern(cx, pattern) ]].into_iter().collect(); From 400cb1411e642c96bc7ae50afc19daff4879663c Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 13 Aug 2018 16:01:13 +0100 Subject: [PATCH 34/47] Add a summary of the algorithm to the file --- src/librustc_mir/hair/pattern/_match.rs | 115 ++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 5af9173be0337..87afa82b6bd23 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -8,6 +8,121 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +/// This file includes the logic for exhaustiveness and usefulness checking for +/// pattern-matching. Specifically, given a list of patterns for a type, we can +/// tell whether: +/// (a) the patterns cover every possible constructor for the type [exhaustiveness] +/// (b) each pattern is necessary [usefulness] +/// +/// The algorithm implemented here is a modified version of the one described in: +/// http://moscova.inria.fr/~maranget/papers/warn/index.html +/// However, to save future implementors from reading the original paper, I'm going +/// to summarise the algorithm here to hopefully save time and be a little clearer +/// (without being so rigorous). +/// +/// The core of the algorithm revolves about a "usefulness" check. In particular, we +/// are trying to compute a predicate `U(P, p_{m + 1})` where `P` is a list of patterns +/// of length `m` for a compound (product) type with `n` components (we refer to this as +/// a matrix). `U(P, p_{m + 1})` represents whether, given an existing list of patterns +/// `p_1 ..= p_m`, adding a new pattern will be "useful" (that is, cover previously- +/// uncovered values of the type). +/// +/// If we have this predicate, then we can easily compute both exhaustiveness of an +/// entire set of patterns and the individual usefulness of each one. +/// (a) the set of patterns is exhaustive iff `U(P, _)` is false (i.e. adding a wildcard +/// match doesn't increase the number of values we're matching) +/// (b) a pattern `p_i` is not useful if `U(P[0..=(i-1), p_i)` is false (i.e. adding a +/// pattern to those that have come before it doesn't increase the number of values +/// we're matching). +/// +/// For example, say we have the following: +/// ``` +/// // x: (Option, Result<()>) +/// match x { +/// (Some(true), _) => {} +/// (None, Err(())) => {} +/// (None, Err(_)) => {} +/// } +/// ``` +/// Here, the matrix `P` is 3 x 2 (rows x columns). +/// [ +/// [Some(true), _], +/// [None, Err(())], +/// [None, Err(_)], +/// ] +/// We can tell it's not exhaustive, because `U(P, _)` is true (we're not covering +/// `[Some(false), _]`, for instance). In addition, row 3 is not useful, because +/// all the values it covers are already covered by row 2. +/// +/// To compute `U`, we must have two other concepts. +/// 1. `S(c, P)` is a "specialised matrix", where `c` is a constructor (like `Some` or +/// `None`). You can think of it as filtering `P` to just the rows whose *first* pattern +/// can cover `c` (and expanding OR-patterns into distinct patterns), and then expanding +/// the constructor into all of its components. +/// +/// It is computed as follows. For each row `p_i` of P, we have four cases: +/// 1.1. `p_(i,1)= c(r_1, .., r_a)`. Then `S(c, P)` has a corresponding row: +/// r_1, .., r_a, p_(i,2), .., p_(i,n) +/// 1.2. `p_(i,1) = c'(r_1, .., r_a')` where `c ≠ c'`. Then `S(c, P)` has no +/// corresponding row. +/// 1.3. `p_(i,1) = _`. Then `S(c, P)` has a corresponding row: +/// _, .., _, p_(i,2), .., p_(i,n) +/// 1.4. `p_(i,1) = r_1 | r_2`. Then `S(c, P)` has corresponding rows inlined from: +/// S(c, (r_1, p_(i,2), .., p_(i,n))) +/// S(c, (r_2, p_(i,2), .., p_(i,n))) +/// +/// 2. `D(P)` is a "default matrix". This is used when we know there are missing +/// constructor cases, but there might be existing wildcard patterns, so to check the +/// usefulness of the matrix, we have to check all its *other* components. +/// +/// It is computed as follows. For each row `p_i` of P, we have three cases: +/// 1.1. `p_(i,1)= c(r_1, .., r_a)`. Then `D(P)` has no corresponding row. +/// 1.2. `p_(i,1) = _`. Then `D(P)` has a corresponding row: +/// p_(i,2), .., p_(i,n) +/// 1.3. `p_(i,1) = r_1 | r_2`. Then `D(P)` has corresponding rows inlined from: +/// D((r_1, p_(i,2), .., p_(i,n))) +/// D((r_2, p_(i,2), .., p_(i,n))) +/// +/// The algorithm for computing `U` +/// ------------------------------- +/// The algorithm is inductive (on the number of columns: i.e. components of tuple patterns). +/// That means we're going to check the components from left-to-right, so the algorithm +/// operates principally on the first component of the matrix and new pattern `p_{m + 1}`. +/// +/// Base case. (`n = 0`, i.e. an empty tuple pattern) +/// - If `P` already contains an empty pattern (i.e. if the number of patterns `m > 0`), +/// then `U(P, p_{m + 1})` is false. +/// - Otherwise, `P` must be empty, so `U(P, p_{m + 1})` is true. +/// +/// Inductive step. (`n > 0`, i.e. 1 or more tuple pattern components) +/// We're going to match on the new pattern, `p_{m + 1}`. +/// - If `p_{m + 1} == c(r_1, .., r_a)`, then we have a constructor pattern. +/// Thus, the usefulness of `p_{m + 1}` can be reduced to whether it is useful when +/// we ignore all the patterns in `P` that involve other constructors. This is where +/// `S(c, P)` comes in: +/// `U(P, p_{m + 1}) := U(S(c, P), S(c, p_{m + 1}))` +/// - If `p_{m + 1} == _`, then we have two more cases: +/// + All the constructors of the first component of the type exist within +/// all the rows (after having expanded OR-patterns). In this case: +/// `U(P, p_{m + 1}) := ∨(k ϵ constructors) U(S(k, P), S(k, p_{m + 1}))` +/// I.e. the pattern `p_{m + 1}` is only useful when all the constructors are +/// present *if* its later components are useful for the respective constructors +/// covered by `p_{m + 1}` (usually a single constructor, but all in the case of `_`). +/// + Some constructors are not present in the existing rows (after having expanded +/// OR-patterns). However, there might be wildcard patterns (`_`) present. Thus, we +/// are only really concerned with the other patterns leading with wildcards. This is +/// where `D` comes in: +/// `U(P, p_{m + 1}) := U(D(P), p_({m + 1},2), .., p_({m + 1},n))` +/// - If `p_{m + 1} == r_1 | r_2`, then the usefulness depends on each separately: +/// `U(P, p_{m + 1}) := U(P, (r_1, p_({m + 1},2), .., p_({m + 1},n))) +/// || U(P, (r_2, p_({m + 1},2), .., p_({m + 1},n)))` +/// +/// Modifications to the algorithm +/// ------------------------------ +/// The algorithm in the paper doesn't cover some of the special cases that arise in Rust, for +/// example uninhabited types and variable-length slice patterns. These are drawn attention to +/// throughout the code below. + use self::Constructor::*; use self::Usefulness::*; use self::WitnessPreference::*; From 9e9e023354a0e9b93e342e5c3578e46c09e318f5 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 13 Aug 2018 16:23:14 +0100 Subject: [PATCH 35/47] More formatting improvements --- src/librustc_mir/hair/pattern/_match.rs | 21 +++++++++----------- src/librustc_mir/hair/pattern/check_match.rs | 2 +- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 87afa82b6bd23..b464b9cb0bf03 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1148,7 +1148,7 @@ fn is_useful_specialized<'p, 'a:'p, 'tcx: 'a>( .collect() ), result => result - }, + } None => NotUseful } } @@ -1167,16 +1167,13 @@ fn pat_constructors<'tcx>(cx: &mut MatchCheckCtxt, -> Option>> { match *pat.kind { - PatternKind::Binding { .. } | PatternKind::Wild => - None, - PatternKind::Leaf { .. } | PatternKind::Deref { .. } => - Some(vec![Single]), - PatternKind::Variant { adt_def, variant_index, .. } => - Some(vec![Variant(adt_def.variants[variant_index].did)]), - PatternKind::Constant { value } => - Some(vec![ConstantValue(value)]), - PatternKind::Range { lo, hi, end } => - Some(vec![ConstantRange(lo, hi, end)]), + PatternKind::Binding { .. } | PatternKind::Wild => None, + PatternKind::Leaf { .. } | PatternKind::Deref { .. } => Some(vec![Single]), + PatternKind::Variant { adt_def, variant_index, .. } => { + Some(vec![Variant(adt_def.variants[variant_index].did)]) + } + PatternKind::Constant { value } => Some(vec![ConstantValue(value)]), + PatternKind::Range { lo, hi, end } => Some(vec![ConstantRange(lo, hi, end)]), PatternKind::Array { .. } => match pcx.ty.sty { ty::TyArray(_, length) => Some(vec![ Slice(length.unwrap_usize(cx.tcx)) @@ -1390,7 +1387,7 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( let head: Option> = match *pat.kind { PatternKind::Binding { .. } | PatternKind::Wild => { Some(wild_patterns.to_owned()) - }, + } PatternKind::Variant { adt_def, variant_index, ref subpatterns, .. } => { let ref variant = adt_def.variants[variant_index]; diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index 3a1e29dfe97b0..7f6298fd17173 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -391,7 +391,7 @@ fn check_arms<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, printed_if_let_err = true; } } - }, + } hir::MatchSource::WhileLetDesugar => { // check which arm we're on. From 527cccb7a7a0213880eb97bd01ff1b9f3c834d5b Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 14 Aug 2018 03:02:31 +0100 Subject: [PATCH 36/47] Add some more compound exhaustiveness tests --- src/test/ui/exhaustive_integer_patterns.rs | 13 +++++++++++++ src/test/ui/exhaustive_integer_patterns.stderr | 8 +++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs index c267c4ef28c08..0f8168cdd04fe 100644 --- a/src/test/ui/exhaustive_integer_patterns.rs +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -138,4 +138,17 @@ fn main() { (1, _) => {} (_, None) => {} } + + match (0u8, true) { //~ ERROR non-exhaustive patterns + (0..=125, false) => {} + (128..=255, false) => {} + (0..=255, true) => {} + } + + match (0u8, true) { + (0..=125, false) => {} + (128..=255, false) => {} + (0..=255, true) => {} + (125..128, false) => {} + } } diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr index 6fabbebf4875a..2222283247b5a 100644 --- a/src/test/ui/exhaustive_integer_patterns.stderr +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -58,6 +58,12 @@ error[E0004]: non-exhaustive patterns: `(0u8, Some(_))` and `(2u8..=255u8, Some( LL | match (0u8, Some(())) { //~ ERROR non-exhaustive patterns | ^^^^^^^^^^^^^^^ patterns `(0u8, Some(_))` and `(2u8..=255u8, Some(_))` not covered -error: aborting due to 9 previous errors +error[E0004]: non-exhaustive patterns: `(126u8..=127u8, false)` not covered + --> $DIR/exhaustive_integer_patterns.rs:142:11 + | +LL | match (0u8, true) { //~ ERROR non-exhaustive patterns + | ^^^^^^^^^^^ pattern `(126u8..=127u8, false)` not covered + +error: aborting due to 10 previous errors For more information about this error, try `rustc --explain E0004`. From e9c8361cc6433161e9578673ed266fdf5f676049 Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 14 Aug 2018 12:45:26 +0100 Subject: [PATCH 37/47] Add equivalence class splitting for range constructors --- src/librustc_mir/hair/pattern/_match.rs | 343 ++++++++++++++++++------ src/librustc_mir/lib.rs | 1 + 2 files changed, 267 insertions(+), 77 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index b464b9cb0bf03..eef3330c0357e 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -59,6 +59,7 @@ /// `None`). You can think of it as filtering `P` to just the rows whose *first* pattern /// can cover `c` (and expanding OR-patterns into distinct patterns), and then expanding /// the constructor into all of its components. +/// The specialisation of a row vector is computed by `specialize`. /// /// It is computed as follows. For each row `p_i` of P, we have four cases: /// 1.1. `p_(i,1)= c(r_1, .., r_a)`. Then `S(c, P)` has a corresponding row: @@ -74,9 +75,10 @@ /// 2. `D(P)` is a "default matrix". This is used when we know there are missing /// constructor cases, but there might be existing wildcard patterns, so to check the /// usefulness of the matrix, we have to check all its *other* components. +/// The default matrix is computed inline in `is_useful`. /// /// It is computed as follows. For each row `p_i` of P, we have three cases: -/// 1.1. `p_(i,1)= c(r_1, .., r_a)`. Then `D(P)` has no corresponding row. +/// 1.1. `p_(i,1) = c(r_1, .., r_a)`. Then `D(P)` has no corresponding row. /// 1.2. `p_(i,1) = _`. Then `D(P)` has a corresponding row: /// p_(i,2), .., p_(i,n) /// 1.3. `p_(i,1) = r_1 | r_2`. Then `D(P)` has corresponding rows inlined from: @@ -88,6 +90,7 @@ /// The algorithm is inductive (on the number of columns: i.e. components of tuple patterns). /// That means we're going to check the components from left-to-right, so the algorithm /// operates principally on the first component of the matrix and new pattern `p_{m + 1}`. +/// This algorithm is realised in the `is_useful` function. /// /// Base case. (`n = 0`, i.e. an empty tuple pattern) /// - If `P` already contains an empty pattern (i.e. if the number of patterns `m > 0`), @@ -101,6 +104,7 @@ /// we ignore all the patterns in `P` that involve other constructors. This is where /// `S(c, P)` comes in: /// `U(P, p_{m + 1}) := U(S(c, P), S(c, p_{m + 1}))` +/// This special case is handled in `is_useful_specialized`. /// - If `p_{m + 1} == _`, then we have two more cases: /// + All the constructors of the first component of the type exist within /// all the rows (after having expanded OR-patterns). In this case: @@ -121,13 +125,48 @@ /// ------------------------------ /// The algorithm in the paper doesn't cover some of the special cases that arise in Rust, for /// example uninhabited types and variable-length slice patterns. These are drawn attention to -/// throughout the code below. +/// throughout the code below. I'll make a quick note here about how exhaustive integer matching +/// is accounted for, though. +/// +/// Exhaustive integer matching +/// --------------------------- +/// An integer type can be thought of as a (huge) sum type: 1 | 2 | 3 | ... +/// So to support exhaustive integer matching, we can make use of the logic in the paper for +/// OR-patterns. However, we obviously can't just treat ranges x..=y as individual sums, because +/// they are likely gigantic. So we instead treat ranges as constructors of the integers. This means +/// that we have a constructor *of* constructors (the integers themselves). We then need to work +/// through all the inductive step rules above, deriving how the ranges would be treated as +/// OR-patterns, and making sure that they're treated in the same way even when they're ranges. +/// There are really only four special cases here: +/// - When we match on a constructor that's actually a range, we have to treat it as if we would +/// an OR-pattern. +/// + It turns out that we can simply extend the case for single-value patterns in +/// `specialize` to either be *equal* to a value constructor, or *contained within* a range +/// constructor. +/// + When the pattern itself is a range, you just want to tell whether any of the values in +/// the pattern range coincide with values in the constructor range, which is precisely +/// intersection. +/// Since when encountering a range pattern for a value constructor, we also use inclusion, it +/// means that whenever the constructor is a value/range and the pattern is also a value/range, +/// we can simply use intersection to test usefulness. +/// - When we're testing for usefulness of a pattern and the pattern's first component is a +/// wildcard. +/// + If all the constructors appear in the matrix, we have a slight complication. By default, +/// the behaviour (i.e. a disjunction over specialised matrices for each constructor) is +/// invalid, because we want a disjunction over every *integer* in each range, not just a +/// disjunction over every range. This is a bit more tricky to deal with: essentially we need +/// to form equivalence classes of subranges of the constructor range for which the behaviour +/// of the matrix `P` and new pattern `p_{m + 1}` are the same. This is described in more +/// detail in `split_grouped_constructors`. +/// + If some constructors are missing from the matrix, it turns out we don't need to do +/// anything special (because we know none of the integers are actually wildcards: i.e. we +/// can't span wildcards using ranges). use self::Constructor::*; use self::Usefulness::*; use self::WitnessPreference::*; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::indexed_vec::Idx; use super::{FieldPattern, Pattern, PatternKind}; @@ -147,7 +186,7 @@ use syntax_pos::{Span, DUMMY_SP}; use arena::TypedArena; -use std::cmp::{self, Ordering}; +use std::cmp::{self, Ordering, min, max}; use std::fmt; use std::iter::{FromIterator, IntoIterator}; use std::ops::RangeInclusive; @@ -800,8 +839,17 @@ impl<'tcx> IntRange<'tcx> { } } - // The return value of `signed_bias` should be - // XORed with an endpoint to encode/decode it. + fn from_pat(tcx: TyCtxt<'_, 'tcx, 'tcx>, + pat: &Pattern<'tcx>) + -> Option> { + Self::from_ctor(tcx, &match pat.kind { + box PatternKind::Constant { value } => ConstantValue(value), + box PatternKind::Range { lo, hi, end } => ConstantRange(lo, hi, end), + _ => return None, + }) + } + + // The return value of `signed_bias` should be XORed with an endpoint to encode/decode it. fn signed_bias(tcx: TyCtxt<'_, 'tcx, 'tcx>, ty: Ty<'tcx>) -> u128 { match ty.sty { ty::TyInt(ity) => { @@ -812,6 +860,24 @@ impl<'tcx> IntRange<'tcx> { } } + /// Convert a `RangeInclusive` to a `ConstantValue` or inclusive `ConstantRange`. + fn range_to_ctor( + tcx: TyCtxt<'_, 'tcx, 'tcx>, + ty: Ty<'tcx>, + r: RangeInclusive, + ) -> Constructor<'tcx> { + let bias = IntRange::signed_bias(tcx, ty); + let ty = ty::ParamEnv::empty().and(ty); + let (lo, hi) = r.into_inner(); + if lo == hi { + ConstantValue(ty::Const::from_bits(tcx, lo ^ bias, ty)) + } else { + ConstantRange(ty::Const::from_bits(tcx, lo ^ bias, ty), + ty::Const::from_bits(tcx, hi ^ bias, ty), + RangeEnd::Included) + } + } + /// Given an `IntRange` corresponding to a pattern in a `match` and a collection of /// ranges corresponding to the domain of values of a type (say, an integer), return /// a new collection of ranges corresponding to the original ranges minus the ranges @@ -823,42 +889,41 @@ impl<'tcx> IntRange<'tcx> { let ranges = ranges.into_iter().filter_map(|r| { IntRange::from_ctor(tcx, &r).map(|i| i.range) }); - // Convert a `RangeInclusive` to a `ConstantValue` or inclusive `ConstantRange`. - let bias = IntRange::signed_bias(tcx, self.ty); - let ty = ty::ParamEnv::empty().and(self.ty); - let range_to_constant = |r: RangeInclusive| { - let (lo, hi) = r.into_inner(); - if lo == hi { - ConstantValue(ty::Const::from_bits(tcx, lo ^ bias, ty)) - } else { - ConstantRange(ty::Const::from_bits(tcx, lo ^ bias, ty), - ty::Const::from_bits(tcx, hi ^ bias, ty), - RangeEnd::Included) - } - }; let mut remaining_ranges = vec![]; + let ty = self.ty; let (lo, hi) = self.range.into_inner(); for subrange in ranges { let (subrange_lo, subrange_hi) = subrange.into_inner(); 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(range_to_constant(subrange_lo..=subrange_hi)); + remaining_ranges.push(Self::range_to_ctor(tcx, ty, subrange_lo..=subrange_hi)); } else { if lo > subrange_lo { // The pattern intersects an upper section of the // subrange, so a lower section will remain. - remaining_ranges.push(range_to_constant(subrange_lo..=(lo - 1))); + remaining_ranges.push(Self::range_to_ctor(tcx, ty, subrange_lo..=(lo - 1))); } if hi < subrange_hi { // The pattern intersects a lower section of the // subrange, so an upper section will remain. - remaining_ranges.push(range_to_constant((hi + 1)..=subrange_hi)); + remaining_ranges.push(Self::range_to_ctor(tcx, ty, (hi + 1)..=subrange_hi)); } } } remaining_ranges } + + fn intersection(&self, other: &Self) -> Option { + let ty = self.ty; + 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 }) + } else { + None + } + } } /// Algorithm from http://moscova.inria.fr/~maranget/papers/warn/index.html @@ -937,7 +1002,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, if let Some(constructors) = pat_constructors(cx, v[0], pcx) { debug!("is_useful - expanding constructors: {:#?}", constructors); - constructors.into_iter().map(|c| + split_grouped_constructors(cx.tcx, constructors, matrix, v, pcx.ty).into_iter().map(|c| is_useful_specialized(cx, matrix, v, c.clone(), pcx.ty, witness) ).find(|result| result.is_useful()).unwrap_or(NotUseful) } else { @@ -952,16 +1017,6 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let all_ctors = all_constructors(cx, pcx); debug!("all_ctors = {:#?}", all_ctors); - // The only constructor patterns for which it is valid to - // treat the values as constructors are ranges (see - // `all_constructors` for details). - let exhaustive_integer_patterns = cx.tcx.features().exhaustive_integer_patterns; - let consider_value_constructors = exhaustive_integer_patterns - && all_ctors.iter().all(|ctor| match ctor { - ConstantRange(..) => true, - _ => false, - }); - // `missing_ctors` are those that should have appeared // as patterns in the `match` expression, but did not. let mut missing_ctors = vec![]; @@ -972,7 +1027,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // If a constructor appears in a `match` arm, we can // eliminate it straight away. refined_ctors = vec![] - } else if exhaustive_integer_patterns { + } else if cx.tcx.features().exhaustive_integer_patterns { if let Some(interval) = IntRange::from_ctor(cx.tcx, used_ctor) { // Refine the required constructors for the type by subtracting // the range defined by the current constructor pattern. @@ -1025,15 +1080,9 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let is_non_exhaustive = is_privately_empty || is_declared_nonexhaustive; if missing_ctors.is_empty() && !is_non_exhaustive { - if consider_value_constructors { - // If we've successfully matched every value - // of the type, then we're done. - NotUseful - } else { - all_ctors.into_iter().map(|c| { - is_useful_specialized(cx, matrix, v, c.clone(), pcx.ty, witness) - }).find(|result| result.is_useful()).unwrap_or(NotUseful) - } + split_grouped_constructors(cx.tcx, all_ctors, matrix, v, pcx.ty).into_iter().map(|c| { + is_useful_specialized(cx, matrix, v, c.clone(), pcx.ty, witness) + }).find(|result| result.is_useful()).unwrap_or(NotUseful) } else { let matrix = rows.iter().filter_map(|r| { if r[0].is_wildcard() { @@ -1119,14 +1168,16 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, } } +/// A shorthand for the `U(S(c, P), S(c, q))` operation from the paper. I.e. `is_useful` applied +/// to the specialised version of both the pattern matrix `P` and the new pattern `q`. fn is_useful_specialized<'p, 'a:'p, 'tcx: 'a>( cx: &mut MatchCheckCtxt<'a, 'tcx>, &Matrix(ref m): &Matrix<'p, 'tcx>, v: &[&'p Pattern<'tcx>], ctor: Constructor<'tcx>, lty: Ty<'tcx>, - witness: WitnessPreference) -> Usefulness<'tcx> -{ + witness: WitnessPreference, +) -> Usefulness<'tcx> { debug!("is_useful_specialized({:#?}, {:#?}, {:?})", v, ctor, lty); let sub_pat_tys = constructor_sub_pattern_tys(cx, &ctor, lty); let wild_patterns_owned: Vec<_> = sub_pat_tys.iter().map(|ty| { @@ -1309,13 +1360,160 @@ fn slice_pat_covered_by_constructor<'tcx>( Ok(true) } +/// For exhaustive integer matching, some constructors are grouped within other constructors +/// (namely integer typed values are grouped within ranges). However, when specialising these +/// constructors, we want to be specialising for the underlying constructors (the integers), not +/// the groups (the ranges). Thus we need to split the groups up. Splitting them up naïvely would +/// mean creating a separate constructor for every single value in the range, which is clearly +/// impractical. However, observe that for some ranges of integers, the specialisation will be +/// identical across all values in that range (i.e. there are equivalence classes of ranges of +/// constructors based on their `is_useful_specialised` outcome). These classes are grouped by +/// the patterns that apply to them (both in the matrix `P` and in the new row `p_{m + 1}`). We +/// can split the range whenever the patterns that apply to that range (specifically: the patterns +/// that *intersect* with that range) change. +/// Our solution, therefore, is to split the range constructor into subranges at every single point +/// the group of intersecting patterns changes, which we can compute by converting each pattern to +/// a range and recording its endpoints, then creating subranges between each consecutive pair of +/// endpoints. +/// And voilà! We're testing precisely those ranges that we need to, without any exhaustive matching +/// on actual integers. The nice thing about this is that the number of subranges is linear in the +/// number of rows in the matrix (i.e. the number of cases in the `match` statement), so we don't +/// need to be worried about matching over gargantuan ranges. +fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( + tcx: TyCtxt<'a, 'tcx, 'tcx>, + ctors: Vec>, + &Matrix(ref m): &Matrix<'p, 'tcx>, + p: &[&'p Pattern<'tcx>], + ty: Ty<'tcx>, +) -> Vec> { + let pat = &p[0]; + + let mut split_ctors = Vec::with_capacity(ctors.len()); + + for ctor in ctors.into_iter() { + match ctor { + // For now, only ranges may denote groups of "subconstructors", so we only need to + // special-case constant ranges. + ConstantRange(..) => { + // We only care about finding all the subranges within the range of the intersection + // of the new pattern `p_({m + 1},1)` (here `pat`) and the constructor range. + // Anything else is irrelevant, because it is guaranteed to result in `NotUseful`, + // which is the default case anyway, and can be ignored. + let mut ctor_range = IntRange::from_ctor(tcx, &ctor).unwrap(); + if let Some(pat_range) = IntRange::from_pat(tcx, pat) { + if let Some(new_range) = ctor_range.intersection(&pat_range) { + ctor_range = new_range; + } else { + // If the intersection between `pat` and the constructor is empty, the + // entire range is `NotUseful`. + continue; + } + } else { + match pat.kind { + box PatternKind::Wild => { + // A wild pattern matches the entire range of values, + // so the current values are fine. + } + // If the pattern is not a value (i.e. a degenerate range), a range or a + // wildcard (which stands for the entire range), then it's guaranteed to + // be `NotUseful`. + _ => continue, + } + } + // We're going to collect all the endpoints in the new pattern so we can create + // subranges between them. + let mut points = FxHashSet::default(); + let (lo, hi) = (*ctor_range.range.start(), *ctor_range.range.end()); + points.insert(lo); + points.insert(hi); + // We're going to iterate through every row pattern, adding endpoints in. + for row in m.iter() { + if let Some(r) = IntRange::from_pat(tcx, row[0]) { + // We're only interested in endpoints that lie (at least partially) + // within the subrange domain. + if let Some(r) = ctor_range.intersection(&r) { + let (r_lo, r_hi) = r.range.into_inner(); + // Insert the endpoints. + points.insert(r_lo); + points.insert(r_hi); + // There's a slight subtlety here, which involves the fact we're using + // inclusive ranges everywhere. When we subdivide the range into + // subranges, they can't overlap, or the subranges effectively + // coalesce. We need hard boundaries between subranges. The simplest + // way to do this is by adding extra "boundary points" to prevent this + // intersection. Technically this means we occasionally check a few more + // cases for usefulness than we need to (because they're part of another + // equivalence class), but it's still linear and very simple to verify, + // which is handy when it comes to matching, which can often be quite + // fiddly. + if r_lo > lo { + points.insert(r_lo - 1); + } + if r_hi < hi { + points.insert(r_hi + 1); + } + } + } + } + + // The patterns were iterated in an arbitrary order (i.e. in the order the user + // wrote them), so we need to make sure our endpoints are sorted. + let mut points: Vec<_> = points.into_iter().collect(); + points.sort(); + let mut points = points.into_iter(); + let mut start = points.next().unwrap(); + // Iterate through pairs of points, adding the subranges to `split_ctors`. + while let Some(end) = points.next() { + split_ctors.push(IntRange::range_to_ctor(tcx, ty, start..=end)); + start = end; + } + } + // Any other constructor can be used unchanged. + _ => split_ctors.push(ctor), + } + } + + split_ctors +} + +/// Check whether there exists any shared value in either `ctor` or `pat` by intersecting them. +fn constructor_intersects_pattern<'p, 'a: 'p, 'tcx: 'a>( + tcx: TyCtxt<'a, 'tcx, 'tcx>, + ctor: &Constructor<'tcx>, + pat: &'p Pattern<'tcx>, +) -> Option>> { + let mut integer_matching = false; + if let ConstantValue(value) | ConstantRange(value, _, _) = ctor { + if let ty::TyChar | ty::TyInt(_) | ty::TyUint(_) = value.ty.sty { + integer_matching = true; + } + } + if integer_matching { + match (IntRange::from_ctor(tcx, ctor), IntRange::from_pat(tcx, pat)) { + (Some(ctor), Some(pat)) => ctor.intersection(&pat).map(|_| vec![]), + _ => None, + } + } else { + // Fallback for non-ranges and ranges that involve floating-point numbers, which are not + // conveniently handled by `IntRange`. For these cases, the constructor may not be a range + // so intersection actually devolves into being covered by the pattern. + match constructor_covered_by_range(tcx, ctor, pat) { + Ok(true) => Some(vec![]), + Ok(false) | Err(ErrorReported) => None, + } + } +} + fn constructor_covered_by_range<'a, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, ctor: &Constructor<'tcx>, - from: &'tcx ty::Const<'tcx>, to: &'tcx ty::Const<'tcx>, - end: RangeEnd, - ty: Ty<'tcx>, + pat: &Pattern<'tcx>, ) -> Result { + let (from, to, end, ty) = match pat.kind { + box PatternKind::Constant { value } => (value, value, RangeEnd::Included, value.ty), + box PatternKind::Range { lo, hi, end } => (lo, hi, end, lo.ty), + _ => bug!("`constructor_covered_by_range` called with {:?}", pat), + }; trace!("constructor_covered_by_range {:#?}, {:#?}, {:#?}, {}", ctor, from, to, ty); let cmp_from = |c_from| compare_const_vals(tcx, c_from, from, ty::ParamEnv::empty().and(ty)) .map(|res| res != Ordering::Less); @@ -1379,9 +1577,8 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( cx: &mut MatchCheckCtxt<'a, 'tcx>, r: &[&'p Pattern<'tcx>], constructor: &Constructor<'tcx>, - wild_patterns: &[&'p Pattern<'tcx>]) - -> Option>> -{ + wild_patterns: &[&'p Pattern<'tcx>], +) -> Option>> { let pat = &r[0]; let head: Option> = match *pat.kind { @@ -1432,28 +1629,22 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( } } _ => { - match constructor_covered_by_range( - cx.tcx, - constructor, value, value, RangeEnd::Included, - value.ty, - ) { - Ok(true) => Some(vec![]), - Ok(false) => None, - Err(ErrorReported) => None, - } + // If the constructor is a single value, we add a row to the specialised matrix + // if the pattern is equal to the constructor. If the constructor is a range of + // values, we add a row to the specialised matrix if the pattern is contained + // within the constructor. These two cases (for a single value pattern) can be + // treated as intersection. + constructor_intersects_pattern(cx.tcx, constructor, pat) } } } - PatternKind::Range { lo, hi, ref end } => { - match constructor_covered_by_range( - cx.tcx, - constructor, lo, hi, end.clone(), lo.ty, - ) { - Ok(true) => Some(vec![]), - Ok(false) => None, - Err(ErrorReported) => None, - } + PatternKind::Range { .. } => { + // If the constructor is a single value, we add a row to the specialised matrix if the + // pattern contains the constructor. If the constructor is a range of values, we add a + // row to the specialised matrix if there exists any value that lies both within the + // pattern and the constructor. These two cases reduce to intersection. + constructor_intersects_pattern(cx.tcx, constructor, pat) } PatternKind::Array { ref prefix, ref slice, ref suffix } | @@ -1463,14 +1654,12 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( let pat_len = prefix.len() + suffix.len(); if let Some(slice_count) = wild_patterns.len().checked_sub(pat_len) { if slice_count == 0 || slice.is_some() { - Some( - prefix.iter().chain( - wild_patterns.iter().map(|p| *p) - .skip(prefix.len()) - .take(slice_count) - .chain( - suffix.iter() - )).collect()) + Some(prefix.iter().chain( + wild_patterns.iter().map(|p| *p) + .skip(prefix.len()) + .take(slice_count) + .chain(suffix.iter()) + ).collect()) } else { None } diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index bda80ff562c75..d7bb666a80567 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -36,6 +36,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment! #![feature(unicode_internals)] #![feature(step_trait)] #![feature(slice_concat_ext)] +#![feature(if_while_or_patterns)] #![recursion_limit="256"] From 1dbc78112f29d12175ff56c9271ac47c042a718a Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 14 Aug 2018 16:40:04 +0100 Subject: [PATCH 38/47] Handle equivalence classes of length-1 ranges --- src/librustc_mir/hair/pattern/_match.rs | 87 ++++++++++++++-------- src/test/ui/exhaustive_integer_patterns.rs | 8 +- 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index eef3330c0357e..449e37489769f 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -166,7 +166,7 @@ use self::Constructor::*; use self::Usefulness::*; use self::WitnessPreference::*; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::indexed_vec::Idx; use super::{FieldPattern, Pattern, PatternKind}; @@ -321,7 +321,7 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { tcx, module, pattern_arena: &pattern_arena, - byte_array_map: FxHashMap(), + byte_array_map: FxHashMap::default(), }) } @@ -1422,10 +1422,33 @@ fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( } // We're going to collect all the endpoints in the new pattern so we can create // subranges between them. - let mut points = FxHashSet::default(); + // If there's a single point, we need to identify it as belonging + // to a length-1 range, so it can be treated as an individual + // constructor, rather than as an endpoint. To do this, we keep track of which + // endpoint a point corresponds to. Whenever a point corresponds to both a start + // and an end, then we create a unit range for it. + #[derive(PartialEq, Clone, Copy, Debug)] + enum Endpoint { + Start, + End, + Both, + }; + let mut points = FxHashMap::default(); + let add_endpoint = |points: &mut FxHashMap<_, _>, x, endpoint| { + points.entry(x).and_modify(|ex_x| { + if *ex_x != endpoint { + *ex_x = Endpoint::Both + } + }).or_insert(endpoint); + }; + let add_endpoints = |points: &mut FxHashMap<_, _>, lo, hi| { + // Insert the endpoints, taking care to keep track of to + // which endpoints a point corresponds. + add_endpoint(points, lo, Endpoint::Start); + add_endpoint(points, hi, Endpoint::End); + }; let (lo, hi) = (*ctor_range.range.start(), *ctor_range.range.end()); - points.insert(lo); - points.insert(hi); + add_endpoints(&mut points, lo, hi); // We're going to iterate through every row pattern, adding endpoints in. for row in m.iter() { if let Some(r) = IntRange::from_pat(tcx, row[0]) { @@ -1433,39 +1456,43 @@ fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( // within the subrange domain. if let Some(r) = ctor_range.intersection(&r) { let (r_lo, r_hi) = r.range.into_inner(); - // Insert the endpoints. - points.insert(r_lo); - points.insert(r_hi); - // There's a slight subtlety here, which involves the fact we're using - // inclusive ranges everywhere. When we subdivide the range into - // subranges, they can't overlap, or the subranges effectively - // coalesce. We need hard boundaries between subranges. The simplest - // way to do this is by adding extra "boundary points" to prevent this - // intersection. Technically this means we occasionally check a few more - // cases for usefulness than we need to (because they're part of another - // equivalence class), but it's still linear and very simple to verify, - // which is handy when it comes to matching, which can often be quite - // fiddly. - if r_lo > lo { - points.insert(r_lo - 1); - } - if r_hi < hi { - points.insert(r_hi + 1); - } + add_endpoints(&mut points, r_lo, r_hi); } } } // The patterns were iterated in an arbitrary order (i.e. in the order the user // wrote them), so we need to make sure our endpoints are sorted. - let mut points: Vec<_> = points.into_iter().collect(); - points.sort(); + let mut points: Vec<(u128, Endpoint)> = points.into_iter().collect(); + points.sort_unstable_by_key(|(x, _)| *x); let mut points = points.into_iter(); - let mut start = points.next().unwrap(); + let mut a = points.next().unwrap(); + // Iterate through pairs of points, adding the subranges to `split_ctors`. - while let Some(end) = points.next() { - split_ctors.push(IntRange::range_to_ctor(tcx, ty, start..=end)); - start = end; + // We have to be careful about the orientation of the points as endpoints, to make + // sure we're enumerating precisely the correct ranges. Too few and the matching is + // actually incorrect. Too many and our diagnostics are poorer. This involves some + // case analysis. + while let Some(b) = points.next() { + // a < b (strictly) + if let Endpoint::Both = a.1 { + split_ctors.push(IntRange::range_to_ctor(tcx, ty, a.0..=a.0)); + } + let c = match a.1 { + Endpoint::Start => a.0, + Endpoint::End | Endpoint::Both => a.0 + 1, + }; + let d = match b.1 { + Endpoint::Start | Endpoint::Both => b.0 - 1, + Endpoint::End => b.0, + }; + // In some cases, we won't need an intermediate range between two ranges + // lie immediately adjacent to one another. + if c <= d { + split_ctors.push(IntRange::range_to_ctor(tcx, ty, c..=d)); + } + + a = b; } } // Any other constructor can be used unchanged. diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs index 0f8168cdd04fe..6e2bebc86ad03 100644 --- a/src/test/ui/exhaustive_integer_patterns.rs +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -145,10 +145,16 @@ fn main() { (0..=255, true) => {} } - match (0u8, true) { + match (0u8, true) { // ok (0..=125, false) => {} (128..=255, false) => {} (0..=255, true) => {} (125..128, false) => {} } + + match 0u8 { // ok + 0..2 => {} + 1..=2 => {} + _ => {} + } } From 0383539deddb37c87c3d7babe8c44a19669bbfba Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 14 Aug 2018 19:25:26 +0100 Subject: [PATCH 39/47] Fix handling of floating-point ranges --- src/librustc_mir/hair/pattern/_match.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 449e37489769f..22345f4322c13 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1360,6 +1360,19 @@ fn slice_pat_covered_by_constructor<'tcx>( Ok(true) } +// Whether to evaluate a constructor using exhaustive integer matching. This is true if the +// constructor is a range or constant with an integer type. +fn should_treat_range_exhaustively(tcx: TyCtxt<'_, 'tcx, 'tcx>, ctor: &Constructor<'tcx>) -> bool { + if tcx.features().exhaustive_integer_patterns { + if let ConstantValue(value) | ConstantRange(value, _, _) = ctor { + if let ty::TyChar | ty::TyInt(_) | ty::TyUint(_) = value.ty.sty { + return true; + } + } + } + false +} + /// For exhaustive integer matching, some constructors are grouped within other constructors /// (namely integer typed values are grouped within ranges). However, when specialising these /// constructors, we want to be specialising for the underlying constructors (the integers), not @@ -1394,7 +1407,7 @@ fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( match ctor { // For now, only ranges may denote groups of "subconstructors", so we only need to // special-case constant ranges. - ConstantRange(..) => { + ConstantRange(..) if should_treat_range_exhaustively(tcx, &ctor) => { // We only care about finding all the subranges within the range of the intersection // of the new pattern `p_({m + 1},1)` (here `pat`) and the constructor range. // Anything else is irrelevant, because it is guaranteed to result in `NotUseful`, @@ -1509,13 +1522,7 @@ fn constructor_intersects_pattern<'p, 'a: 'p, 'tcx: 'a>( ctor: &Constructor<'tcx>, pat: &'p Pattern<'tcx>, ) -> Option>> { - let mut integer_matching = false; - if let ConstantValue(value) | ConstantRange(value, _, _) = ctor { - if let ty::TyChar | ty::TyInt(_) | ty::TyUint(_) = value.ty.sty { - integer_matching = true; - } - } - if integer_matching { + if should_treat_range_exhaustively(tcx, ctor) { match (IntRange::from_ctor(tcx, ctor), IntRange::from_pat(tcx, pat)) { (Some(ctor), Some(pat)) => ctor.intersection(&pat).map(|_| vec![]), _ => None, From 798b9ff9d5b74a2f78e81cb3a2528a477f425d9e Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 19 Aug 2018 23:10:18 +0100 Subject: [PATCH 40/47] Tweak comments --- src/librustc_mir/hair/pattern/_match.rs | 87 ++++++++++++++----------- 1 file changed, 49 insertions(+), 38 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 22345f4322c13..87ae9648af65b 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -62,7 +62,7 @@ /// The specialisation of a row vector is computed by `specialize`. /// /// It is computed as follows. For each row `p_i` of P, we have four cases: -/// 1.1. `p_(i,1)= c(r_1, .., r_a)`. Then `S(c, P)` has a corresponding row: +/// 1.1. `p_(i,1) = c(r_1, .., r_a)`. Then `S(c, P)` has a corresponding row: /// r_1, .., r_a, p_(i,2), .., p_(i,n) /// 1.2. `p_(i,1) = c'(r_1, .., r_a')` where `c ≠ c'`. Then `S(c, P)` has no /// corresponding row. @@ -85,6 +85,9 @@ /// D((r_1, p_(i,2), .., p_(i,n))) /// D((r_2, p_(i,2), .., p_(i,n))) /// +/// Note that the OR-patterns are not always used directly in Rust, but are used to derive +/// the exhaustive integer matching rules, so they're written here for posterity. +/// /// The algorithm for computing `U` /// ------------------------------- /// The algorithm is inductive (on the number of columns: i.e. components of tuple patterns). @@ -97,7 +100,8 @@ /// then `U(P, p_{m + 1})` is false. /// - Otherwise, `P` must be empty, so `U(P, p_{m + 1})` is true. /// -/// Inductive step. (`n > 0`, i.e. 1 or more tuple pattern components) +/// Inductive step. (`n > 0`, i.e. whether there's at least one column +/// [which may then be expanded into further columns later]) /// We're going to match on the new pattern, `p_{m + 1}`. /// - If `p_{m + 1} == c(r_1, .., r_a)`, then we have a constructor pattern. /// Thus, the usefulness of `p_{m + 1}` can be reduced to whether it is useful when @@ -926,6 +930,46 @@ impl<'tcx> IntRange<'tcx> { } } +// Find those constructors that are not matched by any non-wildcard patterns in the current column. +fn compute_missing_ctors<'a, 'tcx: 'a>( + tcx: TyCtxt<'a, 'tcx, 'tcx>, + all_ctors: &Vec>, + used_ctors: &Vec>, +) -> Vec> { + let mut missing_ctors = vec![]; + + for req_ctor in all_ctors { + let mut refined_ctors = vec![req_ctor.clone()]; + for used_ctor in used_ctors { + if used_ctor == req_ctor { + // If a constructor appears in a `match` arm, we can + // eliminate it straight away. + refined_ctors = vec![] + } else if tcx.features().exhaustive_integer_patterns { + if let Some(interval) = IntRange::from_ctor(tcx, used_ctor) { + // Refine the required constructors for the type by subtracting + // the range defined by the current constructor pattern. + refined_ctors = interval.subtract_from(tcx, refined_ctors); + } + } + + // If the constructor patterns that have been considered so far + // already cover the entire range of values, then we the + // constructor is not missing, and we can move on to the next one. + if refined_ctors.is_empty() { + break; + } + } + // If a constructor has not been matched, then it is missing. + // We add `refined_ctors` instead of `req_ctor`, because then we can + // provide more detailed error information about precisely which + // ranges have been omitted. + missing_ctors.extend(refined_ctors); + } + + missing_ctors +} + /// 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: @@ -1017,38 +1061,6 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let all_ctors = all_constructors(cx, pcx); debug!("all_ctors = {:#?}", all_ctors); - // `missing_ctors` are those that should have appeared - // as patterns in the `match` expression, but did not. - let mut missing_ctors = vec![]; - for req_ctor in &all_ctors { - let mut refined_ctors = vec![req_ctor.clone()]; - for used_ctor in &used_ctors { - if used_ctor == req_ctor { - // If a constructor appears in a `match` arm, we can - // eliminate it straight away. - refined_ctors = vec![] - } else if cx.tcx.features().exhaustive_integer_patterns { - if let Some(interval) = IntRange::from_ctor(cx.tcx, used_ctor) { - // Refine the required constructors for the type by subtracting - // the range defined by the current constructor pattern. - refined_ctors = interval.subtract_from(cx.tcx, refined_ctors); - } - } - - // If the constructor patterns that have been considered so far - // already cover the entire range of values, then we the - // constructor is not missing, and we can move on to the next one. - if refined_ctors.is_empty() { - break; - } - } - // If a constructor has not been matched, then it is missing. - // We add `refined_ctors` instead of `req_ctor`, because then we can - // provide more detailed error information about precisely which - // ranges have been omitted. - missing_ctors.extend(refined_ctors); - } - // `missing_ctors` is the set of constructors from the same type as the // first column of `matrix` that are matched only by wildcard patterns // from the first column. @@ -1067,11 +1079,10 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // be a privately-empty enum is when the exhaustive_patterns // feature flag is not present, so this is only // needed for that case. + let missing_ctors = compute_missing_ctors(cx.tcx, &all_ctors, &used_ctors); - let is_privately_empty = - all_ctors.is_empty() && !cx.is_uninhabited(pcx.ty); - let is_declared_nonexhaustive = - cx.is_non_exhaustive_enum(pcx.ty) && !cx.is_local(pcx.ty); + let is_privately_empty = all_ctors.is_empty() && !cx.is_uninhabited(pcx.ty); + let is_declared_nonexhaustive = cx.is_non_exhaustive_enum(pcx.ty) && !cx.is_local(pcx.ty); debug!("missing_ctors={:#?} is_privately_empty={:#?} is_declared_nonexhaustive={:#?}", missing_ctors, is_privately_empty, is_declared_nonexhaustive); From 87463c3962a24f4a102374711d268ac30fd5a88d Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 20 Aug 2018 23:16:15 +0100 Subject: [PATCH 41/47] Improve some comments --- src/librustc_mir/hair/pattern/_match.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 87ae9648af65b..39cc9e049f424 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -796,6 +796,9 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( /// For example, the pattern `-128...127i8` is encoded as `0..=255`. /// This makes comparisons and arithmetic on interval endpoints much more /// straightforward. See `signed_bias` for details. +/// +/// `IntRange` is never used to encode an empty range or a "range" that wraps +/// around the (offset) space: i.e. `range.lo <= range.hi`. struct IntRange<'tcx> { pub range: RangeInclusive, pub ty: Ty<'tcx>, @@ -882,10 +885,8 @@ impl<'tcx> IntRange<'tcx> { } } - /// Given an `IntRange` corresponding to a pattern in a `match` and a collection of - /// ranges corresponding to the domain of values of a type (say, an integer), return - /// a new collection of ranges corresponding to the original ranges minus the ranges - /// covered by the `IntRange`. + /// Return a collection of ranges that spans the values covered by `ranges`, subtracted + /// by the values covered by `self`: i.e. `ranges \ self` (in set notation). fn subtract_from(self, tcx: TyCtxt<'_, 'tcx, 'tcx>, ranges: Vec>) @@ -930,7 +931,7 @@ impl<'tcx> IntRange<'tcx> { } } -// Find those constructors that are not matched by any non-wildcard patterns in the current column. +// Return a set of constructors equivalent to `all_ctors \ used_ctors`. fn compute_missing_ctors<'a, 'tcx: 'a>( tcx: TyCtxt<'a, 'tcx, 'tcx>, all_ctors: &Vec>, @@ -1079,6 +1080,9 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // be a privately-empty enum is when the exhaustive_patterns // feature flag is not present, so this is only // needed for that case. + + // Find those constructors that are not matched by any non-wildcard patterns in the + // current column. let missing_ctors = compute_missing_ctors(cx.tcx, &all_ctors, &used_ctors); let is_privately_empty = all_ctors.is_empty() && !cx.is_uninhabited(pcx.ty); From 6e8a625674b6624fec6f7d40f10df27ca0c316bf Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 20 Aug 2018 23:32:01 +0100 Subject: [PATCH 42/47] Remove pattern consideration from split_grouped_constructors --- src/librustc_mir/hair/pattern/_match.rs | 30 ++++--------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 39cc9e049f424..8c6f1d6759f8a 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1047,7 +1047,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, if let Some(constructors) = pat_constructors(cx, v[0], pcx) { debug!("is_useful - expanding constructors: {:#?}", constructors); - split_grouped_constructors(cx.tcx, constructors, matrix, v, pcx.ty).into_iter().map(|c| + split_grouped_constructors(cx.tcx, constructors, matrix, pcx.ty).into_iter().map(|c| is_useful_specialized(cx, matrix, v, c.clone(), pcx.ty, witness) ).find(|result| result.is_useful()).unwrap_or(NotUseful) } else { @@ -1095,7 +1095,7 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let is_non_exhaustive = is_privately_empty || is_declared_nonexhaustive; if missing_ctors.is_empty() && !is_non_exhaustive { - split_grouped_constructors(cx.tcx, all_ctors, matrix, v, pcx.ty).into_iter().map(|c| { + split_grouped_constructors(cx.tcx, all_ctors, matrix, pcx.ty).into_iter().map(|c| { is_useful_specialized(cx, matrix, v, c.clone(), pcx.ty, witness) }).find(|result| result.is_useful()).unwrap_or(NotUseful) } else { @@ -1411,11 +1411,8 @@ fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( tcx: TyCtxt<'a, 'tcx, 'tcx>, ctors: Vec>, &Matrix(ref m): &Matrix<'p, 'tcx>, - p: &[&'p Pattern<'tcx>], ty: Ty<'tcx>, ) -> Vec> { - let pat = &p[0]; - let mut split_ctors = Vec::with_capacity(ctors.len()); for ctor in ctors.into_iter() { @@ -1427,27 +1424,8 @@ fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( // of the new pattern `p_({m + 1},1)` (here `pat`) and the constructor range. // Anything else is irrelevant, because it is guaranteed to result in `NotUseful`, // which is the default case anyway, and can be ignored. - let mut ctor_range = IntRange::from_ctor(tcx, &ctor).unwrap(); - if let Some(pat_range) = IntRange::from_pat(tcx, pat) { - if let Some(new_range) = ctor_range.intersection(&pat_range) { - ctor_range = new_range; - } else { - // If the intersection between `pat` and the constructor is empty, the - // entire range is `NotUseful`. - continue; - } - } else { - match pat.kind { - box PatternKind::Wild => { - // A wild pattern matches the entire range of values, - // so the current values are fine. - } - // If the pattern is not a value (i.e. a degenerate range), a range or a - // wildcard (which stands for the entire range), then it's guaranteed to - // be `NotUseful`. - _ => continue, - } - } + let ctor_range = IntRange::from_ctor(tcx, &ctor).unwrap(); + // We're going to collect all the endpoints in the new pattern so we can create // subranges between them. // If there's a single point, we need to identify it as belonging From c421af995b567322d3f3fdeb1bccec02645cf25c Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 20 Aug 2018 23:59:46 +0100 Subject: [PATCH 43/47] Add assertion to constructor_intersects_pattern --- src/librustc_mir/hair/pattern/_match.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 8c6f1d6759f8a..85e68a12a9714 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1517,7 +1517,14 @@ fn constructor_intersects_pattern<'p, 'a: 'p, 'tcx: 'a>( ) -> Option>> { if should_treat_range_exhaustively(tcx, ctor) { match (IntRange::from_ctor(tcx, ctor), IntRange::from_pat(tcx, pat)) { - (Some(ctor), Some(pat)) => ctor.intersection(&pat).map(|_| vec![]), + (Some(ctor), Some(pat)) => { + ctor.intersection(&pat).map(|_| { + let (pat_lo, pat_hi) = pat.range.into_inner(); + let (ctor_lo, ctor_hi) = ctor.range.into_inner(); + assert!(pat_lo <= ctor_lo && ctor_hi <= pat_hi); + Some(vec![]) + }) + } _ => None, } } else { @@ -1656,21 +1663,18 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( } } _ => { - // If the constructor is a single value, we add a row to the specialised matrix - // if the pattern is equal to the constructor. If the constructor is a range of - // values, we add a row to the specialised matrix if the pattern is contained - // within the constructor. These two cases (for a single value pattern) can be - // treated as intersection. + // If the constructor is a: + // Single value: add a row if the constructor equals the pattern. + // Range: add a row if the constructor contains the pattern. constructor_intersects_pattern(cx.tcx, constructor, pat) } } } PatternKind::Range { .. } => { - // If the constructor is a single value, we add a row to the specialised matrix if the - // pattern contains the constructor. If the constructor is a range of values, we add a - // row to the specialised matrix if there exists any value that lies both within the - // pattern and the constructor. These two cases reduce to intersection. + // If the constructor is a: + // Single value: add a row if the pattern contains the constructor. + // Range: add a row if the constructor intersects the pattern. constructor_intersects_pattern(cx.tcx, constructor, pat) } From 61b6363cb1bb5ce895cd3927f80ec06a41c147de Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 21 Aug 2018 00:16:12 +0100 Subject: [PATCH 44/47] Add more detail to the split_grouped_constructors comment --- src/librustc_mir/hair/pattern/_match.rs | 33 +++++++++++++++++++------ 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 85e68a12a9714..484b820965887 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1396,9 +1396,9 @@ fn should_treat_range_exhaustively(tcx: TyCtxt<'_, 'tcx, 'tcx>, ctor: &Construct /// impractical. However, observe that for some ranges of integers, the specialisation will be /// identical across all values in that range (i.e. there are equivalence classes of ranges of /// constructors based on their `is_useful_specialised` outcome). These classes are grouped by -/// the patterns that apply to them (both in the matrix `P` and in the new row `p_{m + 1}`). We -/// can split the range whenever the patterns that apply to that range (specifically: the patterns -/// that *intersect* with that range) change. +/// the patterns that apply to them (in the matrix `P`). We can split the range whenever the +/// patterns that apply to that range (specifically: the patterns that *intersect* with that range) +/// change. /// Our solution, therefore, is to split the range constructor into subranges at every single point /// the group of intersecting patterns changes, which we can compute by converting each pattern to /// a range and recording its endpoints, then creating subranges between each consecutive pair of @@ -1407,6 +1407,21 @@ fn should_treat_range_exhaustively(tcx: TyCtxt<'_, 'tcx, 'tcx>, ctor: &Construct /// on actual integers. The nice thing about this is that the number of subranges is linear in the /// number of rows in the matrix (i.e. the number of cases in the `match` statement), so we don't /// need to be worried about matching over gargantuan ranges. +/// +/// Essentially, given the first column of a matrix representing ranges, looking like the following: +/// +/// |------| |----------| |-------| || +/// |-------| |-------| |----| || +/// |---------| +/// +/// We truncate the ranges so that they lie inside each range constructor and then split them +/// up into equivalence classes so the ranges are no longer overlapping: +/// +/// |--|--|||-||||--||---|||-------| |-|||| || +/// +/// The logic for determining how to split the ranges is a little involved: we need to make sure +/// that we have a new range for each subrange for which a different set of rows coïncides, but +/// essentially reduces to case analysis on the endpoints of the ranges. fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( tcx: TyCtxt<'a, 'tcx, 'tcx>, ctors: Vec>, @@ -1420,10 +1435,9 @@ fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( // For now, only ranges may denote groups of "subconstructors", so we only need to // special-case constant ranges. ConstantRange(..) if should_treat_range_exhaustively(tcx, &ctor) => { - // We only care about finding all the subranges within the range of the intersection - // of the new pattern `p_({m + 1},1)` (here `pat`) and the constructor range. - // Anything else is irrelevant, because it is guaranteed to result in `NotUseful`, - // which is the default case anyway, and can be ignored. + // We only care about finding all the subranges within the range of the constructor + // range. Anything else is irrelevant, because it is guaranteed to result in + // `NotUseful`, which is the default case anyway, and can be ignored. let ctor_range = IntRange::from_ctor(tcx, &ctor).unwrap(); // We're going to collect all the endpoints in the new pattern so we can create @@ -1479,6 +1493,9 @@ fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( // sure we're enumerating precisely the correct ranges. Too few and the matching is // actually incorrect. Too many and our diagnostics are poorer. This involves some // case analysis. + // In essence, we need to ensure that every time the set of row-ranges that are + // overlapping changes (as we go through the values covered by the ranges), we split + // into a new subrange. while let Some(b) = points.next() { // a < b (strictly) if let Endpoint::Both = a.1 { @@ -1522,7 +1539,7 @@ fn constructor_intersects_pattern<'p, 'a: 'p, 'tcx: 'a>( let (pat_lo, pat_hi) = pat.range.into_inner(); let (ctor_lo, ctor_hi) = ctor.range.into_inner(); assert!(pat_lo <= ctor_lo && ctor_hi <= pat_hi); - Some(vec![]) + vec![] }) } _ => None, From 6a957e172af90befa1af4bd416ea872f6f50b086 Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 21 Aug 2018 21:04:19 +0100 Subject: [PATCH 45/47] Add a test case for u128::MAX - 1 --- src/librustc_mir/hair/pattern/_match.rs | 2 ++ src/test/ui/exhaustive_integer_patterns.rs | 23 +++++++++++-------- .../ui/exhaustive_integer_patterns.stderr | 8 ++++++- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 484b820965887..e928d4e5f03a6 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1501,6 +1501,8 @@ fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( if let Endpoint::Both = a.1 { split_ctors.push(IntRange::range_to_ctor(tcx, ty, a.0..=a.0)); } + // Integer overflow cannot occur here, because only the first point may be + // u128::MIN and only the last may be u128::MAX. let c = match a.1 { Endpoint::Start => a.0, Endpoint::End | Endpoint::Both => a.0 + 1, diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs index 6e2bebc86ad03..50fc825e74e09 100644 --- a/src/test/ui/exhaustive_integer_patterns.rs +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -140,21 +140,26 @@ fn main() { } match (0u8, true) { //~ ERROR non-exhaustive patterns - (0..=125, false) => {} - (128..=255, false) => {} - (0..=255, true) => {} + (0 ..= 125, false) => {} + (128 ..= 255, false) => {} + (0 ..= 255, true) => {} } match (0u8, true) { // ok - (0..=125, false) => {} - (128..=255, false) => {} - (0..=255, true) => {} - (125..128, false) => {} + (0 ..= 125, false) => {} + (128 ..= 255, false) => {} + (0 ..= 255, true) => {} + (125 .. 128, false) => {} } match 0u8 { // ok - 0..2 => {} - 1..=2 => {} + 0 .. 2 => {} + 1 ..= 2 => {} _ => {} } + + const lim: u128 = u128::MAX - 1; + match 0u128 { //~ ERROR non-exhaustive patterns + 0 ..= lim => {} + } } diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr index 2222283247b5a..3a47a09101211 100644 --- a/src/test/ui/exhaustive_integer_patterns.stderr +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -64,6 +64,12 @@ error[E0004]: non-exhaustive patterns: `(126u8..=127u8, false)` not covered LL | match (0u8, true) { //~ ERROR non-exhaustive patterns | ^^^^^^^^^^^ pattern `(126u8..=127u8, false)` not covered -error: aborting due to 10 previous errors +error[E0004]: non-exhaustive patterns: `340282366920938463463374607431768211455u128` not covered + --> $DIR/exhaustive_integer_patterns.rs:162:11 + | +LL | match 0u128 { //~ ERROR non-exhaustive patterns + | ^^^^^ pattern `340282366920938463463374607431768211455u128` not covered + +error: aborting due to 11 previous errors For more information about this error, try `rustc --explain E0004`. From dec55631d9aee00891e97434590290064c1d788e Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 21 Aug 2018 23:27:45 +0100 Subject: [PATCH 46/47] Use a boundary method instead of an endpoint method for split_grouped_constructors --- src/librustc_mir/hair/pattern/_match.rs | 134 ++++++++------------- src/test/ui/exhaustive_integer_patterns.rs | 4 +- 2 files changed, 54 insertions(+), 84 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index e928d4e5f03a6..e47e2899507ee 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -194,6 +194,7 @@ use std::cmp::{self, Ordering, min, max}; use std::fmt; use std::iter::{FromIterator, IntoIterator}; use std::ops::RangeInclusive; +use std::u128; pub fn expand_pattern<'a, 'tcx>(cx: &MatchCheckCtxt<'a, 'tcx>, pat: Pattern<'tcx>) -> &'a Pattern<'tcx> @@ -799,6 +800,7 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>( /// /// `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)] struct IntRange<'tcx> { pub range: RangeInclusive, pub ty: Ty<'tcx>, @@ -1400,9 +1402,7 @@ fn should_treat_range_exhaustively(tcx: TyCtxt<'_, 'tcx, 'tcx>, ctor: &Construct /// patterns that apply to that range (specifically: the patterns that *intersect* with that range) /// change. /// Our solution, therefore, is to split the range constructor into subranges at every single point -/// the group of intersecting patterns changes, which we can compute by converting each pattern to -/// a range and recording its endpoints, then creating subranges between each consecutive pair of -/// endpoints. +/// the group of intersecting patterns changes (using the method described below). /// And voilà! We're testing precisely those ranges that we need to, without any exhaustive matching /// on actual integers. The nice thing about this is that the number of subranges is linear in the /// number of rows in the matrix (i.e. the number of cases in the `match` statement), so we don't @@ -1414,14 +1414,14 @@ fn should_treat_range_exhaustively(tcx: TyCtxt<'_, 'tcx, 'tcx>, ctor: &Construct /// |-------| |-------| |----| || /// |---------| /// -/// We truncate the ranges so that they lie inside each range constructor and then split them -/// up into equivalence classes so the ranges are no longer overlapping: +/// We split the ranges up into equivalence classes so the ranges are no longer overlapping: /// /// |--|--|||-||||--||---|||-------| |-|||| || /// -/// The logic for determining how to split the ranges is a little involved: we need to make sure -/// that we have a new range for each subrange for which a different set of rows coïncides, but -/// essentially reduces to case analysis on the endpoints of the ranges. +/// The logic for determining how to split the ranges is fairly straightforward: we calculate +/// 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.) fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( tcx: TyCtxt<'a, 'tcx, 'tcx>, ctors: Vec>, @@ -1440,84 +1440,54 @@ fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( // `NotUseful`, which is the default case anyway, and can be ignored. let ctor_range = IntRange::from_ctor(tcx, &ctor).unwrap(); - // We're going to collect all the endpoints in the new pattern so we can create - // subranges between them. - // If there's a single point, we need to identify it as belonging - // to a length-1 range, so it can be treated as an individual - // constructor, rather than as an endpoint. To do this, we keep track of which - // endpoint a point corresponds to. Whenever a point corresponds to both a start - // and an end, then we create a unit range for it. - #[derive(PartialEq, Clone, Copy, Debug)] - enum Endpoint { - Start, - End, - Both, - }; - let mut points = FxHashMap::default(); - let add_endpoint = |points: &mut FxHashMap<_, _>, x, endpoint| { - points.entry(x).and_modify(|ex_x| { - if *ex_x != endpoint { - *ex_x = Endpoint::Both - } - }).or_insert(endpoint); - }; - let add_endpoints = |points: &mut FxHashMap<_, _>, lo, hi| { - // Insert the endpoints, taking care to keep track of to - // which endpoints a point corresponds. - add_endpoint(points, lo, Endpoint::Start); - add_endpoint(points, hi, Endpoint::End); - }; - let (lo, hi) = (*ctor_range.range.start(), *ctor_range.range.end()); - add_endpoints(&mut points, lo, hi); - // We're going to iterate through every row pattern, adding endpoints in. - for row in m.iter() { - if let Some(r) = IntRange::from_pat(tcx, row[0]) { - // We're only interested in endpoints that lie (at least partially) - // within the subrange domain. - if let Some(r) = ctor_range.intersection(&r) { - let (r_lo, r_hi) = r.range.into_inner(); - add_endpoints(&mut points, r_lo, r_hi); - } - } + /// Represents a border between 2 integers. Because the intervals spanning borders + /// must be able to cover every integer, we need 2^128 + 1 such borders. + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] + enum Border { + JustBefore(u128), + AfterMax, } - // The patterns were iterated in an arbitrary order (i.e. in the order the user - // wrote them), so we need to make sure our endpoints are sorted. - let mut points: Vec<(u128, Endpoint)> = points.into_iter().collect(); - points.sort_unstable_by_key(|(x, _)| *x); - let mut points = points.into_iter(); - let mut a = points.next().unwrap(); - - // Iterate through pairs of points, adding the subranges to `split_ctors`. - // We have to be careful about the orientation of the points as endpoints, to make - // sure we're enumerating precisely the correct ranges. Too few and the matching is - // actually incorrect. Too many and our diagnostics are poorer. This involves some - // case analysis. - // In essence, we need to ensure that every time the set of row-ranges that are - // overlapping changes (as we go through the values covered by the ranges), we split - // into a new subrange. - while let Some(b) = points.next() { - // a < b (strictly) - if let Endpoint::Both = a.1 { - split_ctors.push(IntRange::range_to_ctor(tcx, ty, a.0..=a.0)); - } - // Integer overflow cannot occur here, because only the first point may be - // u128::MIN and only the last may be u128::MAX. - let c = match a.1 { - Endpoint::Start => a.0, - Endpoint::End | Endpoint::Both => a.0 + 1, - }; - let d = match b.1 { - Endpoint::Start | Endpoint::Both => b.0 - 1, - Endpoint::End => b.0, + // A function for extracting the borders of an integer interval. + fn range_borders(r: IntRange<'_>) -> impl Iterator { + let (lo, hi) = r.range.into_inner(); + let from = Border::JustBefore(lo); + let to = match hi.checked_add(1) { + Some(m) => Border::JustBefore(m), + None => Border::AfterMax, }; - // In some cases, we won't need an intermediate range between two ranges - // lie immediately adjacent to one another. - if c <= d { - split_ctors.push(IntRange::range_to_ctor(tcx, ty, c..=d)); - } + vec![from, to].into_iter() + } - a = b; + // `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, row[0])) + .flat_map(|range| ctor_range.intersection(&range)) + .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(); + + // 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. + for IntRange { range, .. } in borders.windows(2).filter_map(|window| { + match (window[0], window[1]) { + (Border::JustBefore(n), Border::JustBefore(m)) => { + if n < m { + Some(IntRange { range: n..=(m - 1), ty }) + } else { + None + } + } + (Border::JustBefore(n), Border::AfterMax) => { + Some(IntRange { range: n..=u128::MAX, ty }) + } + (Border::AfterMax, _) => None, + } + }) { + split_ctors.push(IntRange::range_to_ctor(tcx, ty, range)); } } // Any other constructor can be used unchanged. diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs index 50fc825e74e09..a8e9e74905c7b 100644 --- a/src/test/ui/exhaustive_integer_patterns.rs +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -158,8 +158,8 @@ fn main() { _ => {} } - const lim: u128 = u128::MAX - 1; + const LIM: u128 = u128::MAX - 1; match 0u128 { //~ ERROR non-exhaustive patterns - 0 ..= lim => {} + 0 ..= LIM => {} } } From 6971c5d55d06c02c8f0b943b3d0b06ef2611c8ac Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 21 Aug 2018 23:55:57 +0100 Subject: [PATCH 47/47] Add some extra edge case tests --- src/librustc_mir/hair/pattern/_match.rs | 3 ++- src/test/ui/exhaustive_integer_patterns.rs | 8 ++++++++ src/test/ui/exhaustive_integer_patterns.stderr | 14 +++++++++++++- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index e47e2899507ee..81f7d439feb25 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1441,7 +1441,8 @@ fn split_grouped_constructors<'p, 'a: 'p, 'tcx: 'a>( let ctor_range = IntRange::from_ctor(tcx, &ctor).unwrap(); /// Represents a border between 2 integers. Because the intervals spanning borders - /// must be able to cover every integer, we need 2^128 + 1 such 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)] enum Border { JustBefore(u128), diff --git a/src/test/ui/exhaustive_integer_patterns.rs b/src/test/ui/exhaustive_integer_patterns.rs index a8e9e74905c7b..7825aaa291286 100644 --- a/src/test/ui/exhaustive_integer_patterns.rs +++ b/src/test/ui/exhaustive_integer_patterns.rs @@ -162,4 +162,12 @@ fn main() { match 0u128 { //~ ERROR non-exhaustive patterns 0 ..= LIM => {} } + + match 0u128 { //~ ERROR non-exhaustive patterns + 0 ..= 4 => {} + } + + match 0u128 { //~ ERROR non-exhaustive patterns + 4 ..= u128::MAX => {} + } } diff --git a/src/test/ui/exhaustive_integer_patterns.stderr b/src/test/ui/exhaustive_integer_patterns.stderr index 3a47a09101211..44b05a12aebd2 100644 --- a/src/test/ui/exhaustive_integer_patterns.stderr +++ b/src/test/ui/exhaustive_integer_patterns.stderr @@ -70,6 +70,18 @@ error[E0004]: non-exhaustive patterns: `340282366920938463463374607431768211455u LL | match 0u128 { //~ ERROR non-exhaustive patterns | ^^^^^ pattern `340282366920938463463374607431768211455u128` not covered -error: aborting due to 11 previous errors +error[E0004]: non-exhaustive patterns: `5u128..=340282366920938463463374607431768211455u128` not covered + --> $DIR/exhaustive_integer_patterns.rs:166:11 + | +LL | match 0u128 { //~ ERROR non-exhaustive patterns + | ^^^^^ pattern `5u128..=340282366920938463463374607431768211455u128` not covered + +error[E0004]: non-exhaustive patterns: `0u128..=3u128` not covered + --> $DIR/exhaustive_integer_patterns.rs:170:11 + | +LL | match 0u128 { //~ ERROR non-exhaustive patterns + | ^^^^^ pattern `0u128..=3u128` not covered + +error: aborting due to 13 previous errors For more information about this error, try `rustc --explain E0004`.