From c75685bef621a062a44110bed52b95cf7ce1beb7 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 7 Nov 2019 12:09:05 +0000 Subject: [PATCH 01/29] Note link between apply/specialize/arity functions --- src/librustc_mir/hair/pattern/_match.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 84a3923c7b294..c7607ae958115 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -786,6 +786,8 @@ impl<'tcx> Constructor<'tcx> { } /// This returns one wildcard pattern for each argument to this constructor. + /// + /// This must be consistent with `apply`, `specialize_one_pattern` and `arity`. fn wildcard_subpatterns<'a>( &self, cx: &MatchCheckCtxt<'a, 'tcx>, @@ -862,6 +864,8 @@ impl<'tcx> Constructor<'tcx> { /// /// For instance, a tuple pattern `(_, 42, Some([]))` has the arity of 3. /// A struct pattern's arity is the number of fields it contains, etc. + /// + /// This must be consistent with `wildcard_subpatterns`, `specialize_one_pattern` and `apply`. fn arity<'a>(&self, cx: &MatchCheckCtxt<'a, 'tcx>, ty: Ty<'tcx>) -> u64 { debug!("Constructor::arity({:#?}, {:?})", self, ty); match self { @@ -883,6 +887,8 @@ impl<'tcx> Constructor<'tcx> { /// Apply a constructor to a list of patterns, yielding a new pattern. `pats` /// must have as many elements as this constructor's arity. /// + /// This must be consistent with `wildcard_subpatterns`, `specialize_one_pattern` and `arity`. + /// /// Examples: /// `self`: `Constructor::Single` /// `ty`: `(u32, u32, u32)` From 8f7ba7a05864d9828e13433aca27f8baa43054fd Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 7 Nov 2019 17:29:56 +0000 Subject: [PATCH 02/29] Clarify conditions for exhaustive integer range matching --- src/librustc_mir/hair/pattern/_match.rs | 37 +++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index c7607ae958115..4e511c17d4f2c 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -630,6 +630,17 @@ impl<'tcx> Constructor<'tcx> { } } + // 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 is_range_and_should_match_exhaustively(&self, tcx: TyCtxt<'tcx>) -> bool { + let ty = match self { + ConstantValue(value, _) => value.ty, + ConstantRange(_, _, ty, _, _) => ty, + _ => return false, + }; + IntRange::should_treat_range_exhaustively(tcx, ty) + } + fn variant_index_for_adt<'a>( &self, cx: &MatchCheckCtxt<'a, 'tcx>, @@ -1280,6 +1291,13 @@ impl<'tcx> IntRange<'tcx> { } } + fn should_treat_range_exhaustively(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { + // Don't treat `usize`/`isize` exhaustively unless the `precise_pointer_size_matching` + // feature is enabled. + IntRange::is_integral(ty) + && (!ty.is_ptr_sized_integral() || tcx.features().precise_pointer_size_matching) + } + #[inline] fn integral_size_and_signed_bias(tcx: TyCtxt<'tcx>, ty: Ty<'_>) -> Option<(Size, u128)> { match ty.kind { @@ -1857,21 +1875,6 @@ fn slice_pat_covered_by_const<'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>, ctor: &Constructor<'tcx>) -> bool { - let ty = match ctor { - ConstantValue(value, _) => value.ty, - ConstantRange(_, _, ty, _, _) => ty, - _ => return false, - }; - if let ty::Char | ty::Int(_) | ty::Uint(_) = ty.kind { - !ty.is_ptr_sized_integral() || tcx.features().precise_pointer_size_matching - } else { - 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 @@ -1923,7 +1926,7 @@ fn split_grouped_constructors<'p, 'tcx>( for ctor in ctors.into_iter() { match ctor { - ConstantRange(..) if should_treat_range_exhaustively(tcx, &ctor) => { + ConstantRange(..) if ctor.is_range_and_should_match_exhaustively(tcx) => { // 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. @@ -2342,7 +2345,7 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( // 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. - if should_treat_range_exhaustively(cx.tcx, constructor) { + if constructor.is_range_and_should_match_exhaustively(cx.tcx) { match ( IntRange::from_ctor(cx.tcx, cx.param_env, constructor), IntRange::from_pat(cx.tcx, cx.param_env, pat), From 3531c52bfe4a1b7c2c284c03f8eefec146698687 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 7 Nov 2019 18:06:48 +0000 Subject: [PATCH 03/29] Factor out range construction in `all_constructors` --- src/librustc_mir/hair/pattern/_match.rs | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 4e511c17d4f2c..bd97ff032f546 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1156,6 +1156,7 @@ fn all_constructors<'a, 'tcx>( pcx: PatCtxt<'tcx>, ) -> Vec> { debug!("all_constructors({:?})", pcx.ty); + let make_range = |start, end| ConstantRange(start, end, pcx.ty, RangeEnd::Included, pcx.span); match pcx.ty.kind { ty::Bool => [true, false] .iter() @@ -1219,20 +1220,8 @@ fn all_constructors<'a, 'tcx>( ty::Char => { vec![ // The valid Unicode Scalar Value ranges. - ConstantRange( - '\u{0000}' as u128, - '\u{D7FF}' as u128, - cx.tcx.types.char, - RangeEnd::Included, - pcx.span, - ), - ConstantRange( - '\u{E000}' as u128, - '\u{10FFFF}' as u128, - cx.tcx.types.char, - RangeEnd::Included, - pcx.span, - ), + make_range('\u{0000}' as u128, '\u{D7FF}' as u128), + make_range('\u{E000}' as u128, '\u{10FFFF}' as u128), ] } ty::Int(_) | ty::Uint(_) @@ -1248,12 +1237,12 @@ fn all_constructors<'a, 'tcx>( let bits = Integer::from_attr(&cx.tcx, SignedInt(ity)).size().bits() as u128; let min = 1u128 << (bits - 1); let max = min - 1; - vec![ConstantRange(min, max, pcx.ty, RangeEnd::Included, pcx.span)] + vec![make_range(min, max)] } ty::Uint(uty) => { let size = Integer::from_attr(&cx.tcx, UnsignedInt(uty)).size(); let max = truncate(u128::max_value(), size); - vec![ConstantRange(0, max, pcx.ty, RangeEnd::Included, pcx.span)] + vec![make_range(0, max)] } _ => { if cx.is_uninhabited(pcx.ty) { From 34ad52e84f79b190a166f71227c312e15e1003a3 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 7 Nov 2019 18:27:50 +0000 Subject: [PATCH 04/29] `pat_constructor` does not need `pcx` anymore --- src/librustc_mir/hair/pattern/_match.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index bd97ff032f546..77635f6ccb9f4 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1632,7 +1632,7 @@ pub fn is_useful<'p, 'a, 'tcx>( debug!("is_useful_expand_first_col: pcx={:#?}, expanding {:#?}", pcx, v.head()); - if let Some(constructor) = pat_constructor(cx, v.head(), pcx) { + if let Some(constructor) = pat_constructor(cx, v.head()) { debug!("is_useful - expanding constructor: {:#?}", constructor); split_grouped_constructors( cx.tcx, @@ -1651,7 +1651,7 @@ pub fn is_useful<'p, 'a, 'tcx>( debug!("is_useful - expanding wildcard"); let used_ctors: Vec> = - matrix.heads().filter_map(|p| pat_constructor(cx, p, pcx)).collect(); + matrix.heads().filter_map(|p| pat_constructor(cx, p)).collect(); 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 `_`). @@ -1756,10 +1756,9 @@ fn is_useful_specialized<'p, 'a, 'tcx>( fn pat_constructor<'tcx>( cx: &mut MatchCheckCtxt<'_, 'tcx>, pat: &Pat<'tcx>, - pcx: PatCtxt<'tcx>, ) -> Option> { match *pat.kind { - PatKind::AscribeUserType { ref subpattern, .. } => pat_constructor(cx, subpattern, pcx), + PatKind::AscribeUserType { ref subpattern, .. } => pat_constructor(cx, subpattern), PatKind::Binding { .. } | PatKind::Wild => None, PatKind::Leaf { .. } | PatKind::Deref { .. } => Some(Single), PatKind::Variant { adt_def, variant_index, .. } => { @@ -1773,9 +1772,9 @@ fn pat_constructor<'tcx>( end, pat.span, )), - PatKind::Array { .. } => match pcx.ty.kind { + PatKind::Array { .. } => match pat.ty.kind { ty::Array(_, length) => Some(FixedLenSlice(length.eval_usize(cx.tcx, cx.param_env))), - _ => span_bug!(pat.span, "bad ty {:?} for array pattern", pcx.ty), + _ => span_bug!(pat.span, "bad ty {:?} for array pattern", pat.ty), }, PatKind::Slice { ref prefix, ref slice, ref suffix } => { let prefix = prefix.len() as u64; From 8c1835dc6e92aacc1172d7ae4071de1f9dbb0d89 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 7 Nov 2019 18:37:10 +0000 Subject: [PATCH 05/29] IntRange::from_pat is redundant with pat_constructors --- src/librustc_mir/hair/pattern/_match.rs | 42 ++++++++----------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 77635f6ccb9f4..2f2680e8d9b4a 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1374,29 +1374,10 @@ impl<'tcx> IntRange<'tcx> { fn from_pat( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, - mut pat: &Pat<'tcx>, + pat: &Pat<'tcx>, ) -> Option> { - loop { - match pat.kind { - box PatKind::Constant { value } => { - return Self::from_const(tcx, param_env, value, pat.span); - } - box PatKind::Range(PatRange { lo, hi, end }) => { - return Self::from_range( - tcx, - lo.eval_bits(tcx, param_env, lo.ty), - hi.eval_bits(tcx, param_env, hi.ty), - &lo.ty, - &end, - pat.span, - ); - } - box PatKind::AscribeUserType { ref subpattern, .. } => { - pat = subpattern; - } - _ => return None, - } - } + let ctor = pat_constructor(tcx, param_env, pat)?; + IntRange::from_ctor(tcx, param_env, &ctor) } // The return value of `signed_bias` should be XORed with an endpoint to encode/decode it. @@ -1632,7 +1613,7 @@ pub fn is_useful<'p, 'a, 'tcx>( debug!("is_useful_expand_first_col: pcx={:#?}, expanding {:#?}", pcx, v.head()); - if let Some(constructor) = pat_constructor(cx, v.head()) { + if let Some(constructor) = pat_constructor(cx.tcx, cx.param_env, v.head()) { debug!("is_useful - expanding constructor: {:#?}", constructor); split_grouped_constructors( cx.tcx, @@ -1651,7 +1632,7 @@ pub fn is_useful<'p, 'a, 'tcx>( debug!("is_useful - expanding wildcard"); let used_ctors: Vec> = - matrix.heads().filter_map(|p| pat_constructor(cx, p)).collect(); + matrix.heads().filter_map(|p| pat_constructor(cx.tcx, cx.param_env, p)).collect(); 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 `_`). @@ -1754,11 +1735,14 @@ fn is_useful_specialized<'p, 'a, 'tcx>( /// Determines the constructor that the given pattern can be specialized to. /// Returns `None` in case of a catch-all, which can't be specialized. fn pat_constructor<'tcx>( - cx: &mut MatchCheckCtxt<'_, 'tcx>, + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, pat: &Pat<'tcx>, ) -> Option> { match *pat.kind { - PatKind::AscribeUserType { ref subpattern, .. } => pat_constructor(cx, subpattern), + PatKind::AscribeUserType { ref subpattern, .. } => { + pat_constructor(tcx, param_env, subpattern) + } PatKind::Binding { .. } | PatKind::Wild => None, PatKind::Leaf { .. } | PatKind::Deref { .. } => Some(Single), PatKind::Variant { adt_def, variant_index, .. } => { @@ -1766,14 +1750,14 @@ fn pat_constructor<'tcx>( } PatKind::Constant { value } => Some(ConstantValue(value, pat.span)), PatKind::Range(PatRange { lo, hi, end }) => Some(ConstantRange( - lo.eval_bits(cx.tcx, cx.param_env, lo.ty), - hi.eval_bits(cx.tcx, cx.param_env, hi.ty), + lo.eval_bits(tcx, param_env, lo.ty), + hi.eval_bits(tcx, param_env, hi.ty), lo.ty, end, pat.span, )), PatKind::Array { .. } => match pat.ty.kind { - ty::Array(_, length) => Some(FixedLenSlice(length.eval_usize(cx.tcx, cx.param_env))), + ty::Array(_, length) => Some(FixedLenSlice(length.eval_usize(tcx, param_env))), _ => span_bug!(pat.span, "bad ty {:?} for array pattern", pat.ty), }, PatKind::Slice { ref prefix, ref slice, ref suffix } => { From f3752ee5d5777ed68231710a6410823f24417bdc Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 7 Nov 2019 19:15:59 +0000 Subject: [PATCH 06/29] Cleanup `constructor_covered_by_range` --- src/librustc_mir/hair/pattern/_match.rs | 80 +++++++++---------------- 1 file changed, 29 insertions(+), 51 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 2f2680e8d9b4a..87d5795484e3a 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -2137,57 +2137,34 @@ fn constructor_covered_by_range<'tcx>( param_env: ty::ParamEnv<'tcx>, ctor: &Constructor<'tcx>, pat: &Pat<'tcx>, -) -> Result { - let (from, to, end, ty) = match pat.kind { - box PatKind::Constant { value } => (value, value, RangeEnd::Included, value.ty), - box PatKind::Range(PatRange { lo, hi, end }) => (lo, hi, end, lo.ty), +) -> bool { + if let Single = ctor { + return true; + } + + let (pat_from, pat_to, pat_end, ty) = match *pat.kind { + PatKind::Constant { value } => (value, value, RangeEnd::Included, value.ty), + PatKind::Range(PatRange { 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, param_env, ty).map(|res| res != Ordering::Less) + let from_bits = |bits| ty::Const::from_bits(tcx, bits, ty::ParamEnv::empty().and(ty)); + let (ctor_from, ctor_to, ctor_end) = match *ctor { + ConstantValue(value, _) => (value, value, RangeEnd::Included), + ConstantRange(from, to, _, ctor_end, _) => (from_bits(from), from_bits(to), ctor_end), + _ => bug!("`constructor_covered_by_range` called with {:?}", ctor), }; - let cmp_to = |c_to| compare_const_vals(tcx, c_to, to, param_env, ty); - macro_rules! some_or_ok { - ($e:expr) => { - match $e { - Some(to) => to, - None => return Ok(false), // not char or int - } - }; - } - match *ctor { - ConstantValue(value, _) => { - let to = some_or_ok!(cmp_to(value)); - let end = - (to == Ordering::Less) || (end == RangeEnd::Included && to == Ordering::Equal); - Ok(some_or_ok!(cmp_from(value)) && end) - } - ConstantRange(from, to, ty, RangeEnd::Included, _) => { - let to = - some_or_ok!(cmp_to(ty::Const::from_bits(tcx, to, ty::ParamEnv::empty().and(ty),))); - let end = - (to == Ordering::Less) || (end == RangeEnd::Included && to == Ordering::Equal); - Ok(some_or_ok!(cmp_from(ty::Const::from_bits( - tcx, - from, - ty::ParamEnv::empty().and(ty), - ))) && end) - } - ConstantRange(from, to, ty, RangeEnd::Excluded, _) => { - let to = - some_or_ok!(cmp_to(ty::Const::from_bits(tcx, to, ty::ParamEnv::empty().and(ty)))); - let end = - (to == Ordering::Less) || (end == RangeEnd::Excluded && to == Ordering::Equal); - Ok(some_or_ok!(cmp_from(ty::Const::from_bits( - tcx, - from, - ty::ParamEnv::empty().and(ty) - ))) && end) - } - Single => Ok(true), - _ => bug!(), - } + trace!("constructor_covered_by_range {:#?}, {:#?}, {:#?}, {}", ctor, pat_from, pat_to, ty); + + let to = match compare_const_vals(tcx, ctor_to, pat_to, param_env, ty) { + Some(to) => to, + None => return false, + }; + let from = match compare_const_vals(tcx, ctor_from, pat_from, param_env, ty) { + Some(from) => from, + None => return false, + }; + (from == Ordering::Greater || from == Ordering::Equal) + && (to == Ordering::Less || (pat_end == ctor_end && to == Ordering::Equal)) } fn patterns_for_variant<'p, 'a: 'p, 'tcx>( @@ -2336,9 +2313,10 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( // 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(cx.tcx, cx.param_env, constructor, pat) { - Ok(true) => Some(PatStack::default()), - Ok(false) | Err(ErrorReported) => None, + if constructor_covered_by_range(cx.tcx, cx.param_env, constructor, pat) { + Some(PatStack::default()) + } else { + None } } } From 81db2ee902566dfe8d1324f7849ea202028f68fd Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 7 Nov 2019 18:52:13 +0000 Subject: [PATCH 07/29] Special-case range inclusion when the range is integral but non-exhaustive --- src/librustc_mir/hair/pattern/_match.rs | 27 +++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 87d5795484e3a..4f4d814dee96f 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -641,6 +641,15 @@ impl<'tcx> Constructor<'tcx> { IntRange::should_treat_range_exhaustively(tcx, ty) } + fn is_integral_range(&self) -> bool { + let ty = match self { + ConstantValue(value, _) => value.ty, + ConstantRange(_, _, ty, _, _) => ty, + _ => return false, + }; + IntRange::is_integral(ty) + } + fn variant_index_for_adt<'a>( &self, cx: &MatchCheckCtxt<'a, 'tcx>, @@ -1471,6 +1480,12 @@ impl<'tcx> IntRange<'tcx> { } } + fn is_subrange(&self, other: &Self) -> bool { + let (lo, hi) = (*self.range.start(), *self.range.end()); + let (other_lo, other_hi) = (*other.range.start(), *other.range.end()); + other_lo <= lo && hi <= other_hi + } + fn suspicious_intersection(&self, other: &Self) -> bool { // `false` in the following cases: // 1 ---- // 1 ---------- // 1 ---- // 1 ---- @@ -2300,6 +2315,8 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( IntRange::from_pat(cx.tcx, cx.param_env, pat), ) { (Some(ctor), Some(pat)) => ctor.intersection(&pat).map(|_| { + // Constructor splitting should ensure that all intersections we encounter + // are actually inclusions. 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); @@ -2307,6 +2324,16 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( }), _ => None, } + } else if constructor.is_integral_range() { + // If we have an integer range that should not be matched exhaustively, fallback to + // checking for inclusion. + match ( + IntRange::from_ctor(cx.tcx, cx.param_env, constructor), + IntRange::from_pat(cx.tcx, cx.param_env, pat), + ) { + (Some(ctor), Some(pat)) if ctor.is_subrange(&pat) => Some(PatStack::default()), + _ => None, + } } else { // Fallback for non-ranges and ranges that involve // floating-point numbers, which are not conveniently handled From 0a186103ae501f74bdee25270f7e52d0f44694ba Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 7 Nov 2019 19:46:14 +0000 Subject: [PATCH 08/29] Move range exhaustiveness check to IntRange::intersection Only IntRange should need to worry about range exhaustiveness really. --- src/librustc_mir/hair/pattern/_match.rs | 50 +++++++------------------ 1 file changed, 14 insertions(+), 36 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 4f4d814dee96f..a8404940be81d 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -630,17 +630,6 @@ impl<'tcx> Constructor<'tcx> { } } - // 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 is_range_and_should_match_exhaustively(&self, tcx: TyCtxt<'tcx>) -> bool { - let ty = match self { - ConstantValue(value, _) => value.ty, - ConstantRange(_, _, ty, _, _) => ty, - _ => return false, - }; - IntRange::should_treat_range_exhaustively(tcx, ty) - } - fn is_integral_range(&self) -> bool { let ty = match self { ConstantValue(value, _) => value.ty, @@ -1468,24 +1457,23 @@ impl<'tcx> IntRange<'tcx> { remaining_ranges } - fn intersection(&self, other: &Self) -> Option { + fn intersection(&self, tcx: TyCtxt<'tcx>, 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 { - let span = other.span; - Some(IntRange { range: max(lo, other_lo)..=min(hi, other_hi), ty, span }) + if Self::should_treat_range_exhaustively(tcx, ty) { + if lo <= other_hi && other_lo <= hi { + let span = other.span; + Some(IntRange { range: max(lo, other_lo)..=min(hi, other_hi), ty, span }) + } else { + None + } } else { - None + // If the range sould not be treated exhaustively, fallback to checking for inclusion. + if other_lo <= lo && hi <= other_hi { Some(self.clone()) } else { None } } } - fn is_subrange(&self, other: &Self) -> bool { - let (lo, hi) = (*self.range.start(), *self.range.end()); - let (other_lo, other_hi) = (*other.range.start(), *other.range.end()); - other_lo <= lo && hi <= other_hi - } - fn suspicious_intersection(&self, other: &Self) -> bool { // `false` in the following cases: // 1 ---- // 1 ---------- // 1 ---- // 1 ---- @@ -1913,7 +1901,7 @@ fn split_grouped_constructors<'p, 'tcx>( for ctor in ctors.into_iter() { match ctor { - ConstantRange(..) if ctor.is_range_and_should_match_exhaustively(tcx) => { + ConstantRange(..) if IntRange::should_treat_range_exhaustively(tcx, ty) => { // 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. @@ -1951,7 +1939,7 @@ fn split_grouped_constructors<'p, 'tcx>( IntRange::from_pat(tcx, param_env, row.head()).map(|r| (r, row.len())) }) .flat_map(|(range, row_len)| { - let intersection = ctor_range.intersection(&range); + let intersection = ctor_range.intersection(tcx, &range); let should_lint = ctor_range.suspicious_intersection(&range); if let (Some(range), 1, true) = (&intersection, row_len, should_lint) { // FIXME: for now, only check for overlapping ranges on simple range @@ -2309,12 +2297,12 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( // 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. - if constructor.is_range_and_should_match_exhaustively(cx.tcx) { + if constructor.is_integral_range() { match ( IntRange::from_ctor(cx.tcx, cx.param_env, constructor), IntRange::from_pat(cx.tcx, cx.param_env, pat), ) { - (Some(ctor), Some(pat)) => ctor.intersection(&pat).map(|_| { + (Some(ctor), Some(pat)) => ctor.intersection(cx.tcx, &pat).map(|_| { // Constructor splitting should ensure that all intersections we encounter // are actually inclusions. let (pat_lo, pat_hi) = pat.range.into_inner(); @@ -2324,16 +2312,6 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( }), _ => None, } - } else if constructor.is_integral_range() { - // If we have an integer range that should not be matched exhaustively, fallback to - // checking for inclusion. - match ( - IntRange::from_ctor(cx.tcx, cx.param_env, constructor), - IntRange::from_pat(cx.tcx, cx.param_env, pat), - ) { - (Some(ctor), Some(pat)) if ctor.is_subrange(&pat) => Some(PatStack::default()), - _ => None, - } } else { // Fallback for non-ranges and ranges that involve // floating-point numbers, which are not conveniently handled From 6de7afdf9529ec66bc2fabe7ce012a885cd48002 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 7 Nov 2019 20:03:50 +0000 Subject: [PATCH 09/29] Prefer IntRange::into_ctor to range_to_ctor --- src/librustc_mir/hair/pattern/_match.rs | 81 +++++++++++-------------- 1 file changed, 34 insertions(+), 47 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index a8404940be81d..1b589d5ac84da 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1389,20 +1389,15 @@ impl<'tcx> IntRange<'tcx> { } } - /// Converts a `RangeInclusive` to a `ConstantValue` or inclusive `ConstantRange`. - fn range_to_ctor( - tcx: TyCtxt<'tcx>, - ty: Ty<'tcx>, - r: RangeInclusive, - span: Span, - ) -> Constructor<'tcx> { - let bias = IntRange::signed_bias(tcx, ty); - let (lo, hi) = r.into_inner(); + /// Converts an `IntRange` to a `ConstantValue` or inclusive `ConstantRange`. + fn into_ctor(self, tcx: TyCtxt<'tcx>) -> Constructor<'tcx> { + let bias = IntRange::signed_bias(tcx, self.ty); + let (lo, hi) = self.range.into_inner(); if lo == hi { - let ty = ty::ParamEnv::empty().and(ty); - ConstantValue(ty::Const::from_bits(tcx, lo ^ bias, ty), span) + let ty = ty::ParamEnv::empty().and(self.ty); + ConstantValue(ty::Const::from_bits(tcx, lo ^ bias, ty), self.span) } else { - ConstantRange(lo ^ bias, hi ^ bias, ty, RangeEnd::Included, span) + ConstantRange(lo ^ bias, hi ^ bias, self.ty, RangeEnd::Included, self.span) } } @@ -1419,38 +1414,27 @@ impl<'tcx> IntRange<'tcx> { .filter_map(|r| IntRange::from_ctor(tcx, param_env, &r).map(|i| i.range)); let mut remaining_ranges = vec![]; let ty = self.ty; + let span = self.span; 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(Self::range_to_ctor( - tcx, - ty, - subrange_lo..=subrange_hi, - self.span, - )); + remaining_ranges + .push(IntRange { range: subrange_lo..=subrange_hi, ty, span }.into_ctor(tcx)); } else { if lo > subrange_lo { // The pattern intersects an upper section of the // subrange, so a lower section will remain. - remaining_ranges.push(Self::range_to_ctor( - tcx, - ty, - subrange_lo..=(lo - 1), - self.span, - )); + remaining_ranges + .push(IntRange { range: subrange_lo..=(lo - 1), ty, span }.into_ctor(tcx)); } if hi < subrange_hi { // The pattern intersects a lower section of the // subrange, so an upper section will remain. - remaining_ranges.push(Self::range_to_ctor( - tcx, - ty, - (hi + 1)..=subrange_hi, - self.span, - )); + remaining_ranges + .push(IntRange { range: (hi + 1)..=subrange_hi, ty, span }.into_ctor(tcx)); } } } @@ -1964,23 +1948,24 @@ fn split_grouped_constructors<'p, 'tcx>( // We're going to iterate through every adjacent 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, span }) - } else { - None + split_ctors.extend( + 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, span }) + } else { + None + } } - } - (Border::JustBefore(n), Border::AfterMax) => { - Some(IntRange { range: n..=u128::MAX, ty, span }) - } - (Border::AfterMax, _) => None, - }) - { - split_ctors.push(IntRange::range_to_ctor(tcx, ty, range, span)); - } + (Border::JustBefore(n), Border::AfterMax) => { + Some(IntRange { range: n..=u128::MAX, ty, span }) + } + (Border::AfterMax, _) => None, + }) + .map(|range| range.into_ctor(tcx)), + ); } VarLenSlice(self_prefix, self_suffix) => { // The exhaustiveness-checking paper does not include any details on @@ -2127,7 +2112,9 @@ fn lint_overlapping_patterns( int_range.span, &format!( "this range overlaps on `{}`", - IntRange::range_to_ctor(tcx, ty, int_range.range, DUMMY_SP).display(tcx), + IntRange { range: int_range.range, ty, span: DUMMY_SP } + .into_ctor(tcx) + .display(tcx), ), ); } From c6c862574de4245698452f46025163434a9fbbe1 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 09:43:15 +0000 Subject: [PATCH 10/29] Special-case subtracting from a range if that range is not an IntRange --- src/librustc_mir/hair/pattern/_match.rs | 57 ++++++++++++++++--------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 1b589d5ac84da..b84af467d0c22 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -763,31 +763,46 @@ impl<'tcx> Constructor<'tcx> { remaining_ctors } ConstantRange(..) | ConstantValue(..) => { - let mut remaining_ctors = vec![self.clone()]; - for other_ctor in other_ctors { - if other_ctor == self { - // If a constructor appears in a `match` arm, we can - // eliminate it straight away. - remaining_ctors = vec![] - } else if let Some(interval) = IntRange::from_ctor(tcx, param_env, other_ctor) { - // Refine the required constructors for the type by subtracting - // the range defined by the current constructor pattern. - remaining_ctors = interval.subtract_from(tcx, param_env, remaining_ctors); + if let Some(_self_range) = IntRange::from_ctor(tcx, param_env, self) { + let mut remaining_ctors = vec![self.clone()]; + for other_ctor in other_ctors { + if other_ctor == self { + // If a constructor appears in a `match` arm, we can + // eliminate it straight away. + remaining_ctors = vec![] + } else if let Some(interval) = + IntRange::from_ctor(tcx, param_env, other_ctor) + { + // Refine the required constructors for the type by subtracting + // the range defined by the current constructor pattern. + remaining_ctors = + interval.subtract_from(tcx, param_env, remaining_ctors); + } + + // If the constructor patterns that have been considered so far + // already cover the entire range of values, then we know the + // constructor is not missing, and we can move on to the next one. + if remaining_ctors.is_empty() { + break; + } } - // If the constructor patterns that have been considered so far - // already cover the entire range of values, then we know the - // constructor is not missing, and we can move on to the next one. - if remaining_ctors.is_empty() { - break; + // If a constructor has not been matched, then it is missing. + // We add `remaining_ctors` instead of `self`, because then we can + // provide more detailed error information about precisely which + // ranges have been omitted. + remaining_ctors + } else { + if other_ctors.iter().any(|c| { + c == self + // FIXME(Nadrieril): This condition looks fishy + || IntRange::from_ctor(tcx, param_env, c).is_some() + }) { + vec![] + } else { + vec![self.clone()] } } - - // If a constructor has not been matched, then it is missing. - // We add `remaining_ctors` instead of `self`, because then we can - // provide more detailed error information about precisely which - // ranges have been omitted. - remaining_ctors } // This constructor is never covered by anything else NonExhaustive => vec![NonExhaustive], From 75a50886aa17e4b22841c9eafdd3506b3b92e62b Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 10:03:18 +0000 Subject: [PATCH 11/29] Avoid converting through Constructor when subtracting ranges --- src/librustc_mir/hair/pattern/_match.rs | 67 +++++++++++-------------- 1 file changed, 29 insertions(+), 38 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index b84af467d0c22..37bd50579448e 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -763,35 +763,30 @@ impl<'tcx> Constructor<'tcx> { remaining_ctors } ConstantRange(..) | ConstantValue(..) => { - if let Some(_self_range) = IntRange::from_ctor(tcx, param_env, self) { - let mut remaining_ctors = vec![self.clone()]; - for other_ctor in other_ctors { - if other_ctor == self { - // If a constructor appears in a `match` arm, we can + if let Some(self_range) = IntRange::from_ctor(tcx, param_env, self) { + let mut remaining_ranges = vec![self_range.clone()]; + let other_ranges = other_ctors + .into_iter() + .filter_map(|c| IntRange::from_ctor(tcx, param_env, c)); + for other_range in other_ranges { + if other_range == self_range { + // If the `self` range appears directly in a `match` arm, we can // eliminate it straight away. - remaining_ctors = vec![] - } else if let Some(interval) = - IntRange::from_ctor(tcx, param_env, other_ctor) - { - // Refine the required constructors for the type by subtracting - // the range defined by the current constructor pattern. - remaining_ctors = - interval.subtract_from(tcx, param_env, remaining_ctors); + remaining_ranges = vec![]; + } else { + // Otherwise explicitely compute the remaining ranges. + remaining_ranges = other_range.subtract_from(remaining_ranges); } - // If the constructor patterns that have been considered so far - // already cover the entire range of values, then we know the - // constructor is not missing, and we can move on to the next one. - if remaining_ctors.is_empty() { + // If the ranges that have been considered so far already cover the entire + // range of values, we can return early. + if remaining_ranges.is_empty() { break; } } - // If a constructor has not been matched, then it is missing. - // We add `remaining_ctors` instead of `self`, because then we can - // provide more detailed error information about precisely which - // ranges have been omitted. - remaining_ctors + // Convert the ranges back into constructors + remaining_ranges.into_iter().map(|range| range.into_ctor(tcx)).collect() } else { if other_ctors.iter().any(|c| { c == self @@ -1418,38 +1413,27 @@ impl<'tcx> IntRange<'tcx> { /// Returns 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>, - param_env: ty::ParamEnv<'tcx>, - ranges: Vec>, - ) -> Vec> { - let ranges = ranges - .into_iter() - .filter_map(|r| IntRange::from_ctor(tcx, param_env, &r).map(|i| i.range)); + fn subtract_from(self, ranges: Vec>) -> Vec> { let mut remaining_ranges = vec![]; let ty = self.ty; let span = self.span; let (lo, hi) = self.range.into_inner(); for subrange in ranges { - let (subrange_lo, subrange_hi) = subrange.into_inner(); + let (subrange_lo, subrange_hi) = subrange.range.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(IntRange { range: subrange_lo..=subrange_hi, ty, span }.into_ctor(tcx)); + remaining_ranges.push(IntRange { range: subrange_lo..=subrange_hi, ty, span }); } else { if lo > subrange_lo { // The pattern intersects an upper section of the // subrange, so a lower section will remain. - remaining_ranges - .push(IntRange { range: subrange_lo..=(lo - 1), ty, span }.into_ctor(tcx)); + remaining_ranges.push(IntRange { range: subrange_lo..=(lo - 1), ty, span }); } if hi < subrange_hi { // The pattern intersects a lower section of the // subrange, so an upper section will remain. - remaining_ranges - .push(IntRange { range: (hi + 1)..=subrange_hi, ty, span }.into_ctor(tcx)); + remaining_ranges.push(IntRange { range: (hi + 1)..=subrange_hi, ty, span }); } } } @@ -1491,6 +1475,13 @@ impl<'tcx> IntRange<'tcx> { } } +// Ignore spans when comparing, they don't carry semantic information as they are only for lints. +impl<'tcx> std::cmp::PartialEq for IntRange<'tcx> { + fn eq(&self, other: &Self) -> bool { + self.range == other.range && self.ty == other.ty + } +} + // A struct to compute a set of constructors equivalent to `all_ctors \ used_ctors`. struct MissingConstructors<'tcx> { tcx: TyCtxt<'tcx>, From 1909e3f2bf9287e959721229af3680d386ee567b Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 10:31:53 +0000 Subject: [PATCH 12/29] `Constructor::display` was only needed for displaying `IntRange` I'm planning to stop using `ConstantRange`/`ConstantValue` for integral types, so going through `Constructor` will soon not be relevant. --- src/librustc_mir/hair/pattern/_match.rs | 40 +++++++++++-------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 37bd50579448e..1e80dbd2b38d8 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -655,25 +655,6 @@ impl<'tcx> Constructor<'tcx> { } } - fn display(&self, tcx: TyCtxt<'tcx>) -> String { - match self { - Constructor::ConstantValue(val, _) => format!("{}", val), - Constructor::ConstantRange(lo, hi, ty, range_end, _) => { - // Get the right sign on the output: - let ty = ty::ParamEnv::empty().and(*ty); - format!( - "{}{}{}", - ty::Const::from_bits(tcx, *lo, ty), - range_end, - ty::Const::from_bits(tcx, *hi, ty), - ) - } - Constructor::FixedLenSlice(val) => format!("[{}]", val), - Constructor::VarLenSlice(prefix, suffix) => format!("[{}, .., {}]", prefix, suffix), - _ => bug!("bad constructor being displayed: `{:?}", self), - } - } - // Returns the set of constructors covered by `self` but not by // anything in `other_ctors`. fn subtract_ctors( @@ -1473,6 +1454,23 @@ impl<'tcx> IntRange<'tcx> { let (other_lo, other_hi) = (*other.range.start(), *other.range.end()); (lo == other_hi || hi == other_lo) } + + fn display(&self, tcx: TyCtxt<'tcx>) -> String { + let (lo, hi) = (self.range.start(), self.range.end()); + + let bias = IntRange::signed_bias(tcx, self.ty); + let (lo, hi) = (lo ^ bias, hi ^ bias); + + let ty = ty::ParamEnv::empty().and(self.ty); + let lo_const = ty::Const::from_bits(tcx, lo, ty); + let hi_const = ty::Const::from_bits(tcx, hi, ty); + + if lo == hi { + format!("{}", lo_const) + } else { + format!("{}{}{}", lo_const, RangeEnd::Included, hi_const) + } + } } // Ignore spans when comparing, they don't carry semantic information as they are only for lints. @@ -2118,9 +2116,7 @@ fn lint_overlapping_patterns( int_range.span, &format!( "this range overlaps on `{}`", - IntRange { range: int_range.range, ty, span: DUMMY_SP } - .into_ctor(tcx) - .display(tcx), + IntRange { range: int_range.range, ty, span: DUMMY_SP }.display(tcx), ), ); } From f93a70063c5bca73f0cf5fa6fcb54cee7c118b10 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Fri, 15 Nov 2019 16:39:31 +0000 Subject: [PATCH 13/29] Introduce IntRange constructor --- src/librustc_mir/hair/pattern/_match.rs | 36 +++++++++++++++++++------ 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 1e80dbd2b38d8..ce0e12eb95cd0 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -590,7 +590,10 @@ enum Constructor<'tcx> { Variant(DefId), /// Literal values. ConstantValue(&'tcx ty::Const<'tcx>, Span), - /// Ranges of literal values (`2..=5` and `2..5`). + /// Ranges of integer literal values (`2`, `2..=5` or `2..5`). + IntRange(IntRange<'tcx>), + // TODO: non-integer + /// Ranges of literal values (`2.0..=5.2`). ConstantRange(u128, u128, Ty<'tcx>, RangeEnd, Span), /// Array patterns of length `n`. FixedLenSlice(u64), @@ -612,6 +615,7 @@ impl<'tcx> std::cmp::PartialEq for Constructor<'tcx> { Constructor::ConstantRange(a_start, a_end, a_ty, a_range_end, _), Constructor::ConstantRange(b_start, b_end, b_ty, b_range_end, _), ) => a_start == b_start && a_end == b_end && a_ty == b_ty && a_range_end == b_range_end, + (Constructor::IntRange(a), Constructor::IntRange(b)) => a == b, (Constructor::FixedLenSlice(a), Constructor::FixedLenSlice(b)) => a == b, ( Constructor::VarLenSlice(a_prefix, a_suffix), @@ -634,6 +638,7 @@ impl<'tcx> Constructor<'tcx> { let ty = match self { ConstantValue(value, _) => value.ty, ConstantRange(_, _, ty, _, _) => ty, + IntRange(_) => return true, _ => return false, }; IntRange::is_integral(ty) @@ -743,7 +748,7 @@ impl<'tcx> Constructor<'tcx> { remaining_ctors } - ConstantRange(..) | ConstantValue(..) => { + IntRange(..) | ConstantRange(..) | ConstantValue(..) => { if let Some(self_range) = IntRange::from_ctor(tcx, param_env, self) { let mut remaining_ranges = vec![self_range.clone()]; let other_ranges = other_ctors @@ -767,7 +772,7 @@ impl<'tcx> Constructor<'tcx> { } // Convert the ranges back into constructors - remaining_ranges.into_iter().map(|range| range.into_ctor(tcx)).collect() + remaining_ranges.into_iter().map(IntRange).collect() } else { if other_ctors.iter().any(|c| { c == self @@ -855,7 +860,7 @@ impl<'tcx> Constructor<'tcx> { } _ => bug!("bad slice pattern {:?} {:?}", self, ty), }, - ConstantValue(..) | ConstantRange(..) | NonExhaustive => vec![], + ConstantValue(..) | ConstantRange(..) | IntRange(..) | NonExhaustive => vec![], } } @@ -880,7 +885,7 @@ impl<'tcx> Constructor<'tcx> { }, FixedLenSlice(length) => *length, VarLenSlice(prefix, suffix) => prefix + suffix, - ConstantValue(..) | ConstantRange(..) | NonExhaustive => 0, + ConstantValue(..) | ConstantRange(..) | IntRange(..) | NonExhaustive => 0, } } @@ -949,6 +954,10 @@ impl<'tcx> Constructor<'tcx> { hi: ty::Const::from_bits(cx.tcx, hi, ty::ParamEnv::empty().and(ty)), end, }), + IntRange(range) => { + // TODO: do it more directly + return range.clone().into_ctor(cx.tcx).apply(cx, ty, None.into_iter()); + } NonExhaustive => PatKind::Wild, }; @@ -1145,7 +1154,14 @@ fn all_constructors<'a, 'tcx>( pcx: PatCtxt<'tcx>, ) -> Vec> { debug!("all_constructors({:?})", pcx.ty); - let make_range = |start, end| ConstantRange(start, end, pcx.ty, RangeEnd::Included, pcx.span); + let make_range = |start, end| { + IntRange( + // `unwrap()` is ok because we know the type is an integer and the range is + // well-formed. + IntRange::from_range(cx.tcx, start, end, pcx.ty, &RangeEnd::Included, pcx.span) + .unwrap(), + ) + }; match pcx.ty.kind { ty::Bool => [true, false] .iter() @@ -1356,6 +1372,7 @@ impl<'tcx> IntRange<'tcx> { match ctor { ConstantRange(lo, hi, ty, end, span) => Self::from_range(tcx, *lo, *hi, ty, end, *span), ConstantValue(val, span) => Self::from_const(tcx, param_env, val, *span), + IntRange(range) => Some(range.clone()), _ => None, } } @@ -1381,6 +1398,7 @@ impl<'tcx> IntRange<'tcx> { } /// Converts an `IntRange` to a `ConstantValue` or inclusive `ConstantRange`. + /// TODO: Deprecated fn into_ctor(self, tcx: TyCtxt<'tcx>) -> Constructor<'tcx> { let bias = IntRange::signed_bias(tcx, self.ty); let (lo, hi) = self.range.into_inner(); @@ -1889,7 +1907,9 @@ fn split_grouped_constructors<'p, 'tcx>( for ctor in ctors.into_iter() { match ctor { - ConstantRange(..) if IntRange::should_treat_range_exhaustively(tcx, ty) => { + IntRange(..) | ConstantRange(..) + if IntRange::should_treat_range_exhaustively(tcx, ty) => + { // 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. @@ -1968,7 +1988,7 @@ fn split_grouped_constructors<'p, 'tcx>( } (Border::AfterMax, _) => None, }) - .map(|range| range.into_ctor(tcx)), + .map(IntRange), ); } VarLenSlice(self_prefix, self_suffix) => { From 4232816be9d9075e139bd372ab80d17301b6ada7 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Fri, 15 Nov 2019 16:39:56 +0000 Subject: [PATCH 14/29] formatting --- src/librustc_mir/hair/pattern/_match.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index ce0e12eb95cd0..b35e9abd2a47d 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -313,10 +313,11 @@ impl PatternFolder<'tcx> for LiteralExpander<'tcx> { ( &ty::Ref(_, rty, _), &PatKind::Constant { - value: Const { - val: ty::ConstKind::Value(val), - ty: ty::TyS { kind: ty::Ref(_, crty, _), .. } - }, + value: + Const { + val: ty::ConstKind::Value(val), + ty: ty::TyS { kind: ty::Ref(_, crty, _), .. }, + }, }, ) => Pat { ty: pat.ty, @@ -328,7 +329,7 @@ impl PatternFolder<'tcx> for LiteralExpander<'tcx> { kind: box PatKind::Constant { value: self.tcx.mk_const(Const { val: ty::ConstKind::Value( - self.fold_const_value_deref(*val, rty, crty) + self.fold_const_value_deref(*val, rty, crty), ), ty: rty, }), @@ -1314,9 +1315,9 @@ impl<'tcx> IntRange<'tcx> { ) -> Option> { if let Some((target_size, bias)) = Self::integral_size_and_signed_bias(tcx, value.ty) { let ty = value.ty; - let val = if let ty::ConstKind::Value(ConstValue::Scalar( - Scalar::Raw { data, size } - )) = value.val { + let val = if let ty::ConstKind::Value(ConstValue::Scalar(Scalar::Raw { data, size })) = + value.val + { // For this specific pattern we can skip a lot of effort and go // straight to the result, after doing a bit of checking. (We // could remove this branch and just use the next branch, which @@ -2069,10 +2070,10 @@ fn split_grouped_constructors<'p, 'tcx>( max_fixed_len = cmp::max(max_fixed_len, n.eval_usize(tcx, param_env)) } - (ty::ConstKind::Value(ConstValue::Slice { start, end, .. }), - ty::Slice(_)) => { - max_fixed_len = cmp::max(max_fixed_len, (end - start) as u64) - } + ( + ty::ConstKind::Value(ConstValue::Slice { start, end, .. }), + ty::Slice(_), + ) => max_fixed_len = cmp::max(max_fixed_len, (end - start) as u64), _ => {} } } From 6b8bfefa0014bc091acc433f15aa98d37b52e0ba Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 12:01:47 +0000 Subject: [PATCH 15/29] Add `IntRange::to_pat` and use it instead of custom `display()` --- src/librustc_mir/hair/pattern/_match.rs | 31 ++++++++----------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index b35e9abd2a47d..c59f053f59de9 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -956,8 +956,7 @@ impl<'tcx> Constructor<'tcx> { end, }), IntRange(range) => { - // TODO: do it more directly - return range.clone().into_ctor(cx.tcx).apply(cx, ty, None.into_iter()); + return range.to_pat(cx.tcx); } NonExhaustive => PatKind::Wild, }; @@ -1398,19 +1397,6 @@ impl<'tcx> IntRange<'tcx> { } } - /// Converts an `IntRange` to a `ConstantValue` or inclusive `ConstantRange`. - /// TODO: Deprecated - fn into_ctor(self, tcx: TyCtxt<'tcx>) -> Constructor<'tcx> { - let bias = IntRange::signed_bias(tcx, self.ty); - let (lo, hi) = self.range.into_inner(); - if lo == hi { - let ty = ty::ParamEnv::empty().and(self.ty); - ConstantValue(ty::Const::from_bits(tcx, lo ^ bias, ty), self.span) - } else { - ConstantRange(lo ^ bias, hi ^ bias, self.ty, RangeEnd::Included, self.span) - } - } - /// Returns 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, ranges: Vec>) -> Vec> { @@ -1474,7 +1460,7 @@ impl<'tcx> IntRange<'tcx> { (lo == other_hi || hi == other_lo) } - fn display(&self, tcx: TyCtxt<'tcx>) -> String { + fn to_pat(&self, tcx: TyCtxt<'tcx>) -> Pat<'tcx> { let (lo, hi) = (self.range.start(), self.range.end()); let bias = IntRange::signed_bias(tcx, self.ty); @@ -1484,11 +1470,14 @@ impl<'tcx> IntRange<'tcx> { let lo_const = ty::Const::from_bits(tcx, lo, ty); let hi_const = ty::Const::from_bits(tcx, hi, ty); - if lo == hi { - format!("{}", lo_const) + let kind = if lo == hi { + PatKind::Constant { value: lo_const } } else { - format!("{}{}{}", lo_const, RangeEnd::Included, hi_const) - } + PatKind::Range(PatRange { lo: lo_const, hi: hi_const, end: RangeEnd::Included }) + }; + + // This is a brand new pattern, so we don't reuse `self.span`. + Pat { ty: self.ty, span: DUMMY_SP, kind: Box::new(kind) } } } @@ -2137,7 +2126,7 @@ fn lint_overlapping_patterns( int_range.span, &format!( "this range overlaps on `{}`", - IntRange { range: int_range.range, ty, span: DUMMY_SP }.display(tcx), + IntRange { range: int_range.range, ty, span: DUMMY_SP }.to_pat(tcx), ), ); } From 84784dd68e03d83bbf7c79258cd96307e4d931ea Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 13:35:04 +0000 Subject: [PATCH 16/29] Eagerly convert ranges to IntRange That way no `ConstantRange` or `ConstantValue` ever needs to be converted to `IntRange`. --- src/librustc_mir/hair/pattern/_match.rs | 121 ++++++++++++------------ 1 file changed, 63 insertions(+), 58 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index c59f053f59de9..312b414fd2a7a 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -593,8 +593,7 @@ enum Constructor<'tcx> { ConstantValue(&'tcx ty::Const<'tcx>, Span), /// Ranges of integer literal values (`2`, `2..=5` or `2..5`). IntRange(IntRange<'tcx>), - // TODO: non-integer - /// Ranges of literal values (`2.0..=5.2`). + /// Ranges of non-integer literal values (`2.0..=5.2`). ConstantRange(u128, u128, Ty<'tcx>, RangeEnd, Span), /// Array patterns of length `n`. FixedLenSlice(u64), @@ -636,13 +635,10 @@ impl<'tcx> Constructor<'tcx> { } fn is_integral_range(&self) -> bool { - let ty = match self { - ConstantValue(value, _) => value.ty, - ConstantRange(_, _, ty, _, _) => ty, + match self { IntRange(_) => return true, _ => return false, }; - IntRange::is_integral(ty) } fn variant_index_for_adt<'a>( @@ -669,7 +665,7 @@ impl<'tcx> Constructor<'tcx> { param_env: ty::ParamEnv<'tcx>, other_ctors: &Vec>, ) -> Vec> { - match *self { + match self { // Those constructors can only match themselves. Single | Variant(_) => { if other_ctors.iter().any(|c| c == self) { @@ -678,7 +674,7 @@ impl<'tcx> Constructor<'tcx> { vec![self.clone()] } } - FixedLenSlice(self_len) => { + &FixedLenSlice(self_len) => { let overlaps = |c: &Constructor<'_>| match *c { FixedLenSlice(other_len) => other_len == self_len, VarLenSlice(prefix, suffix) => prefix + suffix <= self_len, @@ -749,41 +745,39 @@ impl<'tcx> Constructor<'tcx> { remaining_ctors } - IntRange(..) | ConstantRange(..) | ConstantValue(..) => { - if let Some(self_range) = IntRange::from_ctor(tcx, param_env, self) { - let mut remaining_ranges = vec![self_range.clone()]; - let other_ranges = other_ctors - .into_iter() - .filter_map(|c| IntRange::from_ctor(tcx, param_env, c)); - for other_range in other_ranges { - if other_range == self_range { - // If the `self` range appears directly in a `match` arm, we can - // eliminate it straight away. - remaining_ranges = vec![]; - } else { - // Otherwise explicitely compute the remaining ranges. - remaining_ranges = other_range.subtract_from(remaining_ranges); - } + IntRange(self_range) => { + let mut remaining_ranges = vec![self_range.clone()]; + let other_ranges = + other_ctors.into_iter().filter_map(|c| IntRange::from_ctor(tcx, param_env, c)); + for other_range in other_ranges { + if other_range == *self_range { + // If the `self` range appears directly in a `match` arm, we can + // eliminate it straight away. + remaining_ranges = vec![]; + } else { + // Otherwise explicitely compute the remaining ranges. + remaining_ranges = other_range.subtract_from(remaining_ranges); + } - // If the ranges that have been considered so far already cover the entire - // range of values, we can return early. - if remaining_ranges.is_empty() { - break; - } + // If the ranges that have been considered so far already cover the entire + // range of values, we can return early. + if remaining_ranges.is_empty() { + break; } + } - // Convert the ranges back into constructors - remaining_ranges.into_iter().map(IntRange).collect() + // Convert the ranges back into constructors + remaining_ranges.into_iter().map(IntRange).collect() + } + ConstantRange(..) | ConstantValue(..) => { + if other_ctors.iter().any(|c| { + c == self + // FIXME(Nadrieril): This condition looks fishy + || IntRange::from_ctor(tcx, param_env, c).is_some() + }) { + vec![] } else { - if other_ctors.iter().any(|c| { - c == self - // FIXME(Nadrieril): This condition looks fishy - || IntRange::from_ctor(tcx, param_env, c).is_some() - }) { - vec![] - } else { - vec![self.clone()] - } + vec![self.clone()] } } // This constructor is never covered by anything else @@ -1285,6 +1279,10 @@ impl<'tcx> IntRange<'tcx> { } } + fn is_singleton(&self) -> bool { + self.range.start() == self.range.end() + } + fn should_treat_range_exhaustively(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { // Don't treat `usize`/`isize` exhaustively unless the `precise_pointer_size_matching` // feature is enabled. @@ -1363,15 +1361,13 @@ impl<'tcx> IntRange<'tcx> { } fn from_ctor( - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, + _tcx: TyCtxt<'tcx>, + _param_env: ty::ParamEnv<'tcx>, ctor: &Constructor<'tcx>, ) -> Option> { // Floating-point ranges are permitted and we don't want // to consider them when constructing integer ranges. match ctor { - ConstantRange(lo, hi, ty, end, span) => Self::from_range(tcx, *lo, *hi, ty, end, *span), - ConstantValue(val, span) => Self::from_const(tcx, param_env, val, *span), IntRange(range) => Some(range.clone()), _ => None, } @@ -1747,14 +1743,23 @@ fn pat_constructor<'tcx>( PatKind::Variant { adt_def, variant_index, .. } => { Some(Variant(adt_def.variants[variant_index].def_id)) } - PatKind::Constant { value } => Some(ConstantValue(value, pat.span)), - PatKind::Range(PatRange { lo, hi, end }) => Some(ConstantRange( - lo.eval_bits(tcx, param_env, lo.ty), - hi.eval_bits(tcx, param_env, hi.ty), - lo.ty, - end, - pat.span, - )), + PatKind::Constant { value } => { + if let Some(int_range) = IntRange::from_const(tcx, param_env, value, pat.span) { + Some(IntRange(int_range)) + } else { + Some(ConstantValue(value, pat.span)) + } + } + PatKind::Range(PatRange { lo, hi, end }) => { + let ty = lo.ty; + let lo = lo.eval_bits(tcx, param_env, lo.ty); + let hi = hi.eval_bits(tcx, param_env, hi.ty); + if let Some(int_range) = IntRange::from_range(tcx, lo, hi, ty, &end, pat.span) { + Some(IntRange(int_range)) + } else { + Some(ConstantRange(lo, hi, ty, end, pat.span)) + } + } PatKind::Array { .. } => match pat.ty.kind { ty::Array(_, length) => Some(FixedLenSlice(length.eval_usize(tcx, param_env))), _ => span_bug!(pat.span, "bad ty {:?} for array pattern", pat.ty), @@ -1897,13 +1902,13 @@ fn split_grouped_constructors<'p, 'tcx>( for ctor in ctors.into_iter() { match ctor { - IntRange(..) | ConstantRange(..) - if IntRange::should_treat_range_exhaustively(tcx, ty) => - { - // 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, param_env, &ctor).unwrap(); + IntRange(ctor_range) if IntRange::should_treat_range_exhaustively(tcx, ty) => { + // Fast-track if the range is trivial. In particular, don't do the overlapping + // ranges check. + if ctor_range.is_singleton() { + split_ctors.push(IntRange(ctor_range)); + continue; + } /// Represents a border between 2 integers. Because the intervals spanning borders /// must be able to cover every integer, we need to be able to represent From aadd5e52636c4a48fd515357cad78b9f010074ca Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 20:59:10 +0000 Subject: [PATCH 17/29] Factor out getting the boundaries of an `IntRange` --- src/librustc_mir/hair/pattern/_match.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 312b414fd2a7a..ee18e70528e5d 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1283,6 +1283,10 @@ impl<'tcx> IntRange<'tcx> { self.range.start() == self.range.end() } + fn boundaries(&self) -> (u128, u128) { + (*self.range.start(), *self.range.end()) + } + fn should_treat_range_exhaustively(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { // Don't treat `usize`/`isize` exhaustively unless the `precise_pointer_size_matching` // feature is enabled. @@ -1395,11 +1399,11 @@ impl<'tcx> IntRange<'tcx> { /// Returns 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, ranges: Vec>) -> Vec> { + fn subtract_from(&self, ranges: Vec>) -> Vec> { let mut remaining_ranges = vec![]; let ty = self.ty; let span = self.span; - let (lo, hi) = self.range.into_inner(); + let (lo, hi) = self.boundaries(); for subrange in ranges { let (subrange_lo, subrange_hi) = subrange.range.into_inner(); if lo > subrange_hi || subrange_lo > hi { @@ -1424,8 +1428,8 @@ impl<'tcx> IntRange<'tcx> { fn intersection(&self, tcx: TyCtxt<'tcx>, 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()); + let (lo, hi) = self.boundaries(); + let (other_lo, other_hi) = other.boundaries(); if Self::should_treat_range_exhaustively(tcx, ty) { if lo <= other_hi && other_lo <= hi { let span = other.span; @@ -1451,13 +1455,13 @@ impl<'tcx> IntRange<'tcx> { // `true` in the following cases: // 1 ------- // 1 ------- // 2 -------- // 2 ------- - let (lo, hi) = (*self.range.start(), *self.range.end()); - let (other_lo, other_hi) = (*other.range.start(), *other.range.end()); + let (lo, hi) = self.boundaries(); + let (other_lo, other_hi) = other.boundaries(); (lo == other_hi || hi == other_lo) } fn to_pat(&self, tcx: TyCtxt<'tcx>) -> Pat<'tcx> { - let (lo, hi) = (self.range.start(), self.range.end()); + let (lo, hi) = self.boundaries(); let bias = IntRange::signed_bias(tcx, self.ty); let (lo, hi) = (lo ^ bias, hi ^ bias); @@ -2309,8 +2313,8 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( (Some(ctor), Some(pat)) => ctor.intersection(cx.tcx, &pat).map(|_| { // Constructor splitting should ensure that all intersections we encounter // are actually inclusions. - let (pat_lo, pat_hi) = pat.range.into_inner(); - let (ctor_lo, ctor_hi) = ctor.range.into_inner(); + let (pat_lo, pat_hi) = pat.boundaries(); + let (ctor_lo, ctor_hi) = ctor.boundaries(); assert!(pat_lo <= ctor_lo && ctor_hi <= pat_hi); PatStack::default() }), From d1642f1eb51c349c3caa940da43f0b5a51111011 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 21:01:26 +0000 Subject: [PATCH 18/29] Inline now-trivial IntRange::from_ctor --- src/librustc_mir/hair/pattern/_match.rs | 84 +++++++++---------------- 1 file changed, 28 insertions(+), 56 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index ee18e70528e5d..090bcb8be16ad 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -659,12 +659,7 @@ impl<'tcx> Constructor<'tcx> { // Returns the set of constructors covered by `self` but not by // anything in `other_ctors`. - fn subtract_ctors( - &self, - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, - other_ctors: &Vec>, - ) -> Vec> { + fn subtract_ctors(&self, other_ctors: &Vec>) -> Vec> { match self { // Those constructors can only match themselves. Single | Variant(_) => { @@ -747,22 +742,22 @@ impl<'tcx> Constructor<'tcx> { } IntRange(self_range) => { let mut remaining_ranges = vec![self_range.clone()]; - let other_ranges = - other_ctors.into_iter().filter_map(|c| IntRange::from_ctor(tcx, param_env, c)); - for other_range in other_ranges { - if other_range == *self_range { - // If the `self` range appears directly in a `match` arm, we can - // eliminate it straight away. - remaining_ranges = vec![]; - } else { - // Otherwise explicitely compute the remaining ranges. - remaining_ranges = other_range.subtract_from(remaining_ranges); - } + for other_ctor in other_ctors { + if let IntRange(other_range) = other_ctor { + if other_range == self_range { + // If the `self` range appears directly in a `match` arm, we can + // eliminate it straight away. + remaining_ranges = vec![]; + } else { + // Otherwise explicitely compute the remaining ranges. + remaining_ranges = other_range.subtract_from(remaining_ranges); + } - // If the ranges that have been considered so far already cover the entire - // range of values, we can return early. - if remaining_ranges.is_empty() { - break; + // If the ranges that have been considered so far already cover the entire + // range of values, we can return early. + if remaining_ranges.is_empty() { + break; + } } } @@ -773,7 +768,7 @@ impl<'tcx> Constructor<'tcx> { if other_ctors.iter().any(|c| { c == self // FIXME(Nadrieril): This condition looks fishy - || IntRange::from_ctor(tcx, param_env, c).is_some() + || c.is_integral_range() }) { vec![] } else { @@ -1364,26 +1359,15 @@ impl<'tcx> IntRange<'tcx> { } } - fn from_ctor( - _tcx: TyCtxt<'tcx>, - _param_env: ty::ParamEnv<'tcx>, - ctor: &Constructor<'tcx>, - ) -> Option> { - // Floating-point ranges are permitted and we don't want - // to consider them when constructing integer ranges. - match ctor { - IntRange(range) => Some(range.clone()), - _ => None, - } - } - fn from_pat( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, pat: &Pat<'tcx>, ) -> Option> { - let ctor = pat_constructor(tcx, param_env, pat)?; - IntRange::from_ctor(tcx, param_env, &ctor) + match pat_constructor(tcx, param_env, pat)? { + IntRange(range) => Some(range), + _ => None, + } } // The return value of `signed_bias` should be XORed with an endpoint to encode/decode it. @@ -1490,20 +1474,13 @@ impl<'tcx> std::cmp::PartialEq for IntRange<'tcx> { // A struct to compute a set of constructors equivalent to `all_ctors \ used_ctors`. struct MissingConstructors<'tcx> { - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, all_ctors: Vec>, used_ctors: Vec>, } impl<'tcx> MissingConstructors<'tcx> { - fn new( - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, - all_ctors: Vec>, - used_ctors: Vec>, - ) -> Self { - MissingConstructors { tcx, param_env, all_ctors, used_ctors } + fn new(all_ctors: Vec>, used_ctors: Vec>) -> Self { + MissingConstructors { all_ctors, used_ctors } } fn into_inner(self) -> (Vec>, Vec>) { @@ -1521,9 +1498,7 @@ impl<'tcx> MissingConstructors<'tcx> { /// Iterate over all_ctors \ used_ctors fn iter<'a>(&'a self) -> impl Iterator> + Captures<'a> { - self.all_ctors.iter().flat_map(move |req_ctor| { - req_ctor.subtract_ctors(self.tcx, self.param_env, &self.used_ctors) - }) + self.all_ctors.iter().flat_map(move |req_ctor| req_ctor.subtract_ctors(&self.used_ctors)) } } @@ -1649,7 +1624,7 @@ pub fn is_useful<'p, 'a, 'tcx>( // Missing constructors are those that are not matched by any non-wildcard patterns in the // current column. We only fully construct them on-demand, because they're rarely used and // can be big. - let missing_ctors = MissingConstructors::new(cx.tcx, cx.param_env, all_ctors, used_ctors); + let missing_ctors = MissingConstructors::new(all_ctors, used_ctors); debug!("missing_ctors.empty()={:#?}", missing_ctors.is_empty(),); @@ -2305,12 +2280,9 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( // 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. - if constructor.is_integral_range() { - match ( - IntRange::from_ctor(cx.tcx, cx.param_env, constructor), - IntRange::from_pat(cx.tcx, cx.param_env, pat), - ) { - (Some(ctor), Some(pat)) => ctor.intersection(cx.tcx, &pat).map(|_| { + if let IntRange(ctor) = constructor { + match IntRange::from_pat(cx.tcx, cx.param_env, pat) { + Some(pat) => ctor.intersection(cx.tcx, &pat).map(|_| { // Constructor splitting should ensure that all intersections we encounter // are actually inclusions. let (pat_lo, pat_hi) = pat.boundaries(); From 019212420216a8a4ec6ec9c11cd14cab7e4f9b89 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 21:06:41 +0000 Subject: [PATCH 19/29] Make should_treat_range_exhaustively a method --- src/librustc_mir/hair/pattern/_match.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 090bcb8be16ad..e1cb847c68dec 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1282,11 +1282,10 @@ impl<'tcx> IntRange<'tcx> { (*self.range.start(), *self.range.end()) } - fn should_treat_range_exhaustively(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { + fn treat_exhaustively(&self, tcx: TyCtxt<'tcx>) -> bool { // Don't treat `usize`/`isize` exhaustively unless the `precise_pointer_size_matching` // feature is enabled. - IntRange::is_integral(ty) - && (!ty.is_ptr_sized_integral() || tcx.features().precise_pointer_size_matching) + !self.ty.is_ptr_sized_integral() || tcx.features().precise_pointer_size_matching } #[inline] @@ -1414,7 +1413,7 @@ impl<'tcx> IntRange<'tcx> { let ty = self.ty; let (lo, hi) = self.boundaries(); let (other_lo, other_hi) = other.boundaries(); - if Self::should_treat_range_exhaustively(tcx, ty) { + if self.treat_exhaustively(tcx) { if lo <= other_hi && other_lo <= hi { let span = other.span; Some(IntRange { range: max(lo, other_lo)..=min(hi, other_hi), ty, span }) @@ -1881,7 +1880,7 @@ fn split_grouped_constructors<'p, 'tcx>( for ctor in ctors.into_iter() { match ctor { - IntRange(ctor_range) if IntRange::should_treat_range_exhaustively(tcx, ty) => { + IntRange(ctor_range) if ctor_range.treat_exhaustively(tcx) => { // Fast-track if the range is trivial. In particular, don't do the overlapping // ranges check. if ctor_range.is_singleton() { From f674f89e93c4ae7a8ce315468d3e05b86e92ef24 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 21:09:12 +0000 Subject: [PATCH 20/29] Store Const directly in ConstantRange --- src/librustc_mir/hair/pattern/_match.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index e1cb847c68dec..96d120cdefd7a 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -594,7 +594,7 @@ enum Constructor<'tcx> { /// Ranges of integer literal values (`2`, `2..=5` or `2..5`). IntRange(IntRange<'tcx>), /// Ranges of non-integer literal values (`2.0..=5.2`). - ConstantRange(u128, u128, Ty<'tcx>, RangeEnd, Span), + ConstantRange(&'tcx ty::Const<'tcx>, &'tcx ty::Const<'tcx>, Ty<'tcx>, RangeEnd, Span), /// Array patterns of length `n`. FixedLenSlice(u64), /// Slice patterns. Captures any array constructor of `length >= i + j`. @@ -939,11 +939,7 @@ impl<'tcx> Constructor<'tcx> { PatKind::Slice { prefix, slice: Some(wild), suffix } } &ConstantValue(value, _) => PatKind::Constant { value }, - &ConstantRange(lo, hi, ty, end, _) => PatKind::Range(PatRange { - lo: ty::Const::from_bits(cx.tcx, lo, ty::ParamEnv::empty().and(ty)), - hi: ty::Const::from_bits(cx.tcx, hi, ty::ParamEnv::empty().and(ty)), - end, - }), + &ConstantRange(lo, hi, _, end, _) => PatKind::Range(PatRange { lo, hi, end }), IntRange(range) => { return range.to_pat(cx.tcx); } @@ -1730,9 +1726,14 @@ fn pat_constructor<'tcx>( } PatKind::Range(PatRange { lo, hi, end }) => { let ty = lo.ty; - let lo = lo.eval_bits(tcx, param_env, lo.ty); - let hi = hi.eval_bits(tcx, param_env, hi.ty); - if let Some(int_range) = IntRange::from_range(tcx, lo, hi, ty, &end, pat.span) { + if let Some(int_range) = IntRange::from_range( + tcx, + lo.eval_bits(tcx, param_env, lo.ty), + hi.eval_bits(tcx, param_env, hi.ty), + ty, + &end, + pat.span, + ) { Some(IntRange(int_range)) } else { Some(ConstantRange(lo, hi, ty, end, pat.span)) @@ -2132,10 +2133,9 @@ fn constructor_covered_by_range<'tcx>( PatKind::Range(PatRange { lo, hi, end }) => (lo, hi, end, lo.ty), _ => bug!("`constructor_covered_by_range` called with {:?}", pat), }; - let from_bits = |bits| ty::Const::from_bits(tcx, bits, ty::ParamEnv::empty().and(ty)); let (ctor_from, ctor_to, ctor_end) = match *ctor { ConstantValue(value, _) => (value, value, RangeEnd::Included), - ConstantRange(from, to, _, ctor_end, _) => (from_bits(from), from_bits(to), ctor_end), + ConstantRange(from, to, _, ctor_end, _) => (from, to, ctor_end), _ => bug!("`constructor_covered_by_range` called with {:?}", ctor), }; trace!("constructor_covered_by_range {:#?}, {:#?}, {:#?}, {}", ctor, pat_from, pat_to, ty); From 3e5aadc350c016cdcfba2eaaaa4c4ff141dddab5 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 21:11:59 +0000 Subject: [PATCH 21/29] Remove unnecessary data from ConstantValue/ConstantRange --- src/librustc_mir/hair/pattern/_match.rs | 52 +++++++------------------ 1 file changed, 14 insertions(+), 38 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 96d120cdefd7a..ee5af95152248 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -582,7 +582,7 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] enum Constructor<'tcx> { /// The constructor of all patterns that don't vary by constructor, /// e.g., struct patterns and fixed-length arrays. @@ -590,11 +590,11 @@ enum Constructor<'tcx> { /// Enum variants. Variant(DefId), /// Literal values. - ConstantValue(&'tcx ty::Const<'tcx>, Span), + ConstantValue(&'tcx ty::Const<'tcx>), /// Ranges of integer literal values (`2`, `2..=5` or `2..5`). IntRange(IntRange<'tcx>), /// Ranges of non-integer literal values (`2.0..=5.2`). - ConstantRange(&'tcx ty::Const<'tcx>, &'tcx ty::Const<'tcx>, Ty<'tcx>, RangeEnd, Span), + ConstantRange(&'tcx ty::Const<'tcx>, &'tcx ty::Const<'tcx>, RangeEnd), /// Array patterns of length `n`. FixedLenSlice(u64), /// Slice patterns. Captures any array constructor of `length >= i + j`. @@ -603,29 +603,6 @@ enum Constructor<'tcx> { NonExhaustive, } -// Ignore spans when comparing, they don't carry semantic information as they are only for lints. -impl<'tcx> std::cmp::PartialEq for Constructor<'tcx> { - fn eq(&self, other: &Self) -> bool { - match (self, other) { - (Constructor::Single, Constructor::Single) => true, - (Constructor::NonExhaustive, Constructor::NonExhaustive) => true, - (Constructor::Variant(a), Constructor::Variant(b)) => a == b, - (Constructor::ConstantValue(a, _), Constructor::ConstantValue(b, _)) => a == b, - ( - Constructor::ConstantRange(a_start, a_end, a_ty, a_range_end, _), - Constructor::ConstantRange(b_start, b_end, b_ty, b_range_end, _), - ) => a_start == b_start && a_end == b_end && a_ty == b_ty && a_range_end == b_range_end, - (Constructor::IntRange(a), Constructor::IntRange(b)) => a == b, - (Constructor::FixedLenSlice(a), Constructor::FixedLenSlice(b)) => a == b, - ( - Constructor::VarLenSlice(a_prefix, a_suffix), - Constructor::VarLenSlice(b_prefix, b_suffix), - ) => a_prefix == b_prefix && a_suffix == b_suffix, - _ => false, - } - } -} - impl<'tcx> Constructor<'tcx> { fn is_slice(&self) -> bool { match self { @@ -652,7 +629,7 @@ impl<'tcx> Constructor<'tcx> { assert!(!adt.is_enum()); VariantIdx::new(0) } - ConstantValue(c, _) => crate::const_eval::const_variant_index(cx.tcx, cx.param_env, c), + ConstantValue(c) => crate::const_eval::const_variant_index(cx.tcx, cx.param_env, c), _ => bug!("bad constructor {:?} for adt {:?}", self, adt), } } @@ -938,8 +915,8 @@ impl<'tcx> Constructor<'tcx> { let wild = Pat::wildcard_from_ty(ty); PatKind::Slice { prefix, slice: Some(wild), suffix } } - &ConstantValue(value, _) => PatKind::Constant { value }, - &ConstantRange(lo, hi, _, end, _) => PatKind::Range(PatRange { lo, hi, end }), + &ConstantValue(value) => PatKind::Constant { value }, + &ConstantRange(lo, hi, end) => PatKind::Range(PatRange { lo, hi, end }), IntRange(range) => { return range.to_pat(cx.tcx); } @@ -1148,10 +1125,9 @@ fn all_constructors<'a, 'tcx>( ) }; match pcx.ty.kind { - ty::Bool => [true, false] - .iter() - .map(|&b| ConstantValue(ty::Const::from_bool(cx.tcx, b), pcx.span)) - .collect(), + ty::Bool => { + [true, false].iter().map(|&b| ConstantValue(ty::Const::from_bool(cx.tcx, b))).collect() + } ty::Array(ref sub_ty, len) if len.try_eval_usize(cx.tcx, cx.param_env).is_some() => { let len = len.eval_usize(cx.tcx, cx.param_env); if len != 0 && cx.is_uninhabited(sub_ty) { vec![] } else { vec![FixedLenSlice(len)] } @@ -1721,7 +1697,7 @@ fn pat_constructor<'tcx>( if let Some(int_range) = IntRange::from_const(tcx, param_env, value, pat.span) { Some(IntRange(int_range)) } else { - Some(ConstantValue(value, pat.span)) + Some(ConstantValue(value)) } } PatKind::Range(PatRange { lo, hi, end }) => { @@ -1736,7 +1712,7 @@ fn pat_constructor<'tcx>( ) { Some(IntRange(int_range)) } else { - Some(ConstantRange(lo, hi, ty, end, pat.span)) + Some(ConstantRange(lo, hi, end)) } } PatKind::Array { .. } => match pat.ty.kind { @@ -2134,8 +2110,8 @@ fn constructor_covered_by_range<'tcx>( _ => bug!("`constructor_covered_by_range` called with {:?}", pat), }; let (ctor_from, ctor_to, ctor_end) = match *ctor { - ConstantValue(value, _) => (value, value, RangeEnd::Included), - ConstantRange(from, to, _, ctor_end, _) => (from, to, ctor_end), + ConstantValue(value) => (value, value, RangeEnd::Included), + ConstantRange(from, to, ctor_end) => (from, to, ctor_end), _ => bug!("`constructor_covered_by_range` called with {:?}", ctor), }; trace!("constructor_covered_by_range {:#?}, {:#?}, {:#?}, {}", ctor, pat_from, pat_to, ty); @@ -2331,7 +2307,7 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( None } } - ConstantValue(cv, _) => { + ConstantValue(cv) => { match slice_pat_covered_by_const( cx.tcx, pat.span, From e47d631ca0eebb2e5b7f0d40230f78ba7b92592e Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 21:42:02 +0000 Subject: [PATCH 22/29] Malformed range patterns can't happen thanks to E0030 --- src/librustc_mir/hair/pattern/_match.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index ee5af95152248..e3b0c5a5c69af 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1118,8 +1118,7 @@ fn all_constructors<'a, 'tcx>( debug!("all_constructors({:?})", pcx.ty); let make_range = |start, end| { IntRange( - // `unwrap()` is ok because we know the type is an integer and the range is - // well-formed. + // `unwrap()` is ok because we know the type is an integer. IntRange::from_range(cx.tcx, start, end, pcx.ty, &RangeEnd::Included, pcx.span) .unwrap(), ) @@ -1318,13 +1317,12 @@ impl<'tcx> IntRange<'tcx> { // which makes the interval arithmetic simpler. let bias = IntRange::signed_bias(tcx, ty); let (lo, hi) = (lo ^ bias, hi ^ bias); - // Make sure the interval is well-formed. - if lo > hi || lo == hi && *end == RangeEnd::Excluded { - None - } else { - let offset = (*end == RangeEnd::Excluded) as u128; - Some(IntRange { range: lo..=(hi - offset), ty, span }) + let offset = (*end == RangeEnd::Excluded) as u128; + if lo > hi || (lo == hi && *end == RangeEnd::Excluded) { + // This hould have been caught earlier by E0030 + bug!("malformed range pattern: {}..={}", lo, (hi - offset)); } + Some(IntRange { range: lo..=(hi - offset), ty, span }) } else { None } From d31b4c37506ee7a36f3edeb63deba6de22745ce3 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Nov 2019 21:46:46 +0000 Subject: [PATCH 23/29] Remove fishy condition That condition was leftover from a refactor, and was probably not intended. In fact it can't trigger: it would require a ConstantValue of an integral type for which `try_eval_bits` fails. But since we only apply `subtract_ctors` to the output of `all_ctors`, this won't happen. --- src/librustc_mir/hair/pattern/_match.rs | 26 ++----------------------- 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index e3b0c5a5c69af..be268738da8d2 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -611,13 +611,6 @@ impl<'tcx> Constructor<'tcx> { } } - fn is_integral_range(&self) -> bool { - match self { - IntRange(_) => return true, - _ => return false, - }; - } - fn variant_index_for_adt<'a>( &self, cx: &MatchCheckCtxt<'a, 'tcx>, @@ -639,12 +632,8 @@ impl<'tcx> Constructor<'tcx> { fn subtract_ctors(&self, other_ctors: &Vec>) -> Vec> { match self { // Those constructors can only match themselves. - Single | Variant(_) => { - if other_ctors.iter().any(|c| c == self) { - vec![] - } else { - vec![self.clone()] - } + Single | Variant(_) | ConstantValue(..) | ConstantRange(..) => { + if other_ctors.iter().any(|c| c == self) { vec![] } else { vec![self.clone()] } } &FixedLenSlice(self_len) => { let overlaps = |c: &Constructor<'_>| match *c { @@ -741,17 +730,6 @@ impl<'tcx> Constructor<'tcx> { // Convert the ranges back into constructors remaining_ranges.into_iter().map(IntRange).collect() } - ConstantRange(..) | ConstantValue(..) => { - if other_ctors.iter().any(|c| { - c == self - // FIXME(Nadrieril): This condition looks fishy - || c.is_integral_range() - }) { - vec![] - } else { - vec![self.clone()] - } - } // This constructor is never covered by anything else NonExhaustive => vec![NonExhaustive], } From c38aad4f4b04976f4fa10418e0f71880e01d2960 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 12 Nov 2019 09:42:31 +0000 Subject: [PATCH 24/29] Add test for failing `try_eval_bits` --- .../ui/pattern/usefulness/exhaustive_integer_patterns.rs | 9 +++++++++ .../usefulness/exhaustive_integer_patterns.stderr | 8 +++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs index 59f749198971b..e52c6936f12f5 100644 --- a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs +++ b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs @@ -154,4 +154,13 @@ fn main() { match 0u128 { //~ ERROR non-exhaustive patterns 4 ..= u128::MAX => {} } + + const FOO: i32 = 42; + const BAR: &i32 = &42; + match &0 { + &42 => {} + &FOO => {} //~ ERROR unreachable pattern + BAR => {} // not detected as unreachable because `try_eval_bits` fails on BAR + _ => {} + } } diff --git a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.stderr b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.stderr index 7a3a36a820c65..0fbeb981ea015 100644 --- a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.stderr +++ b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.stderr @@ -118,6 +118,12 @@ LL | match 0u128 { | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms -error: aborting due to 14 previous errors +error: unreachable pattern + --> $DIR/exhaustive_integer_patterns.rs:162:9 + | +LL | &FOO => {} + | ^^^^ + +error: aborting due to 15 previous errors For more information about this error, try `rustc --explain E0004`. From 9165dd056b4a54293b8f3a430653d6ef953af65e Mon Sep 17 00:00:00 2001 From: Nadrieril Feneanar Date: Tue, 12 Nov 2019 12:47:34 +0100 Subject: [PATCH 25/29] Apply suggestions from code review Co-Authored-By: Mazdak Farrokhzad --- src/librustc_mir/hair/pattern/_match.rs | 8 ++++---- .../ui/pattern/usefulness/exhaustive_integer_patterns.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index be268738da8d2..69416b4e6b5e6 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -737,7 +737,7 @@ impl<'tcx> Constructor<'tcx> { /// This returns one wildcard pattern for each argument to this constructor. /// - /// This must be consistent with `apply`, `specialize_one_pattern` and `arity`. + /// This must be consistent with `apply`, `specialize_one_pattern`, and `arity`. fn wildcard_subpatterns<'a>( &self, cx: &MatchCheckCtxt<'a, 'tcx>, @@ -815,7 +815,7 @@ impl<'tcx> Constructor<'tcx> { /// For instance, a tuple pattern `(_, 42, Some([]))` has the arity of 3. /// A struct pattern's arity is the number of fields it contains, etc. /// - /// This must be consistent with `wildcard_subpatterns`, `specialize_one_pattern` and `apply`. + /// This must be consistent with `wildcard_subpatterns`, `specialize_one_pattern`, and `apply`. fn arity<'a>(&self, cx: &MatchCheckCtxt<'a, 'tcx>, ty: Ty<'tcx>) -> u64 { debug!("Constructor::arity({:#?}, {:?})", self, ty); match self { @@ -837,7 +837,7 @@ impl<'tcx> Constructor<'tcx> { /// Apply a constructor to a list of patterns, yielding a new pattern. `pats` /// must have as many elements as this constructor's arity. /// - /// This must be consistent with `wildcard_subpatterns`, `specialize_one_pattern` and `arity`. + /// This must be consistent with `wildcard_subpatterns`, `specialize_one_pattern`, and `arity`. /// /// Examples: /// `self`: `Constructor::Single` @@ -1369,7 +1369,7 @@ impl<'tcx> IntRange<'tcx> { None } } else { - // If the range sould not be treated exhaustively, fallback to checking for inclusion. + // If the range should not be treated exhaustively, fallback to checking for inclusion. if other_lo <= lo && hi <= other_hi { Some(self.clone()) } else { None } } } diff --git a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs index e52c6936f12f5..2dcf4dcaeb83f 100644 --- a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs +++ b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs @@ -160,7 +160,7 @@ fn main() { match &0 { &42 => {} &FOO => {} //~ ERROR unreachable pattern - BAR => {} // not detected as unreachable because `try_eval_bits` fails on BAR + BAR => {} // Not detected as unreachable because `try_eval_bits` fails on `BAR`. _ => {} } } From b679e77f0462e17f5268e22e07499608ad2199e5 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 12 Nov 2019 13:03:05 +0000 Subject: [PATCH 26/29] Factor out IntRange::is_subrange --- src/librustc_mir/hair/pattern/_match.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 69416b4e6b5e6..7f77e2b0c8887 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1357,6 +1357,10 @@ impl<'tcx> IntRange<'tcx> { remaining_ranges } + fn is_subrange(&self, other: &Self) -> bool { + other.range.start() <= self.range.start() && self.range.end() <= other.range.end() + } + fn intersection(&self, tcx: TyCtxt<'tcx>, other: &Self) -> Option { let ty = self.ty; let (lo, hi) = self.boundaries(); @@ -1370,7 +1374,7 @@ impl<'tcx> IntRange<'tcx> { } } else { // If the range should not be treated exhaustively, fallback to checking for inclusion. - if other_lo <= lo && hi <= other_hi { Some(self.clone()) } else { None } + if self.is_subrange(other) { Some(self.clone()) } else { None } } } @@ -2236,9 +2240,7 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( Some(pat) => ctor.intersection(cx.tcx, &pat).map(|_| { // Constructor splitting should ensure that all intersections we encounter // are actually inclusions. - let (pat_lo, pat_hi) = pat.boundaries(); - let (ctor_lo, ctor_hi) = ctor.boundaries(); - assert!(pat_lo <= ctor_lo && ctor_hi <= pat_hi); + assert!(ctor.is_subrange(&pat)); PatStack::default() }), _ => None, From addd8a9003b549a81cacb816a9ebbcd1289c55f9 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 12 Nov 2019 13:23:27 +0000 Subject: [PATCH 27/29] Apply suggestions from code review --- src/librustc_mir/hair/pattern/_match.rs | 32 ++++++++++--------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 7f77e2b0c8887..0e07a5983bab5 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1231,9 +1231,9 @@ impl<'tcx> IntRange<'tcx> { (*self.range.start(), *self.range.end()) } + /// Don't treat `usize`/`isize` exhaustively unless the `precise_pointer_size_matching` feature + /// is enabled. fn treat_exhaustively(&self, tcx: TyCtxt<'tcx>) -> bool { - // Don't treat `usize`/`isize` exhaustively unless the `precise_pointer_size_matching` - // feature is enabled. !self.ty.is_ptr_sized_integral() || tcx.features().precise_pointer_size_matching } @@ -1416,7 +1416,7 @@ impl<'tcx> IntRange<'tcx> { } } -// Ignore spans when comparing, they don't carry semantic information as they are only for lints. +/// Ignore spans when comparing, they don't carry semantic information as they are only for lints. impl<'tcx> std::cmp::PartialEq for IntRange<'tcx> { fn eq(&self, other: &Self) -> bool { self.range == other.range && self.ty == other.ty @@ -2079,9 +2079,9 @@ fn constructor_covered_by_range<'tcx>( param_env: ty::ParamEnv<'tcx>, ctor: &Constructor<'tcx>, pat: &Pat<'tcx>, -) -> bool { +) -> Option<()> { if let Single = ctor { - return true; + return Some(()); } let (pat_from, pat_to, pat_end, ty) = match *pat.kind { @@ -2096,16 +2096,11 @@ fn constructor_covered_by_range<'tcx>( }; trace!("constructor_covered_by_range {:#?}, {:#?}, {:#?}, {}", ctor, pat_from, pat_to, ty); - let to = match compare_const_vals(tcx, ctor_to, pat_to, param_env, ty) { - Some(to) => to, - None => return false, - }; - let from = match compare_const_vals(tcx, ctor_from, pat_from, param_env, ty) { - Some(from) => from, - None => return false, - }; - (from == Ordering::Greater || from == Ordering::Equal) - && (to == Ordering::Less || (pat_end == ctor_end && to == Ordering::Equal)) + let to = compare_const_vals(tcx, ctor_to, pat_to, param_env, ty)?; + let from = compare_const_vals(tcx, ctor_from, pat_from, param_env, ty)?; + let intersects = (from == Ordering::Greater || from == Ordering::Equal) + && (to == Ordering::Less || (pat_end == ctor_end && to == Ordering::Equal)); + if intersects { Some(()) } else { None } } fn patterns_for_variant<'p, 'a: 'p, 'tcx>( @@ -2251,11 +2246,8 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( // by `IntRange`. For these cases, the constructor may not be a // range so intersection actually devolves into being covered // by the pattern. - if constructor_covered_by_range(cx.tcx, cx.param_env, constructor, pat) { - Some(PatStack::default()) - } else { - None - } + constructor_covered_by_range(cx.tcx, cx.param_env, constructor, pat) + .map(|()| PatStack::default()) } } From cde9808eaa07f2cd16b0c8fb0a1ab59bc62d3c6e Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 12 Nov 2019 13:24:04 +0000 Subject: [PATCH 28/29] Add regression test --- .../ui/pattern/usefulness/exhaustive_integer_patterns.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs index 2dcf4dcaeb83f..d379dc44bf10b 100644 --- a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs +++ b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs @@ -163,4 +163,10 @@ fn main() { BAR => {} // Not detected as unreachable because `try_eval_bits` fails on `BAR`. _ => {} } + + // Regression test, see https://github.com/rust-lang/rust/pull/66326#issuecomment-552889933 + match &0 { + BAR => {} // ok + _ => {} + } } From 694a511df5e7473a696b50d30c3428bd0d54f140 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Fri, 15 Nov 2019 17:00:38 +0000 Subject: [PATCH 29/29] Apply suggestions from code review --- src/librustc_mir/hair/pattern/_match.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 0e07a5983bab5..e30d6819d04f1 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -593,8 +593,8 @@ enum Constructor<'tcx> { ConstantValue(&'tcx ty::Const<'tcx>), /// Ranges of integer literal values (`2`, `2..=5` or `2..5`). IntRange(IntRange<'tcx>), - /// Ranges of non-integer literal values (`2.0..=5.2`). - ConstantRange(&'tcx ty::Const<'tcx>, &'tcx ty::Const<'tcx>, RangeEnd), + /// Ranges of floating-point literal values (`2.0..=5.2`). + FloatRange(&'tcx ty::Const<'tcx>, &'tcx ty::Const<'tcx>, RangeEnd), /// Array patterns of length `n`. FixedLenSlice(u64), /// Slice patterns. Captures any array constructor of `length >= i + j`. @@ -632,7 +632,7 @@ impl<'tcx> Constructor<'tcx> { fn subtract_ctors(&self, other_ctors: &Vec>) -> Vec> { match self { // Those constructors can only match themselves. - Single | Variant(_) | ConstantValue(..) | ConstantRange(..) => { + Single | Variant(_) | ConstantValue(..) | FloatRange(..) => { if other_ctors.iter().any(|c| c == self) { vec![] } else { vec![self.clone()] } } &FixedLenSlice(self_len) => { @@ -727,7 +727,7 @@ impl<'tcx> Constructor<'tcx> { } } - // Convert the ranges back into constructors + // Convert the ranges back into constructors. remaining_ranges.into_iter().map(IntRange).collect() } // This constructor is never covered by anything else @@ -805,7 +805,7 @@ impl<'tcx> Constructor<'tcx> { } _ => bug!("bad slice pattern {:?} {:?}", self, ty), }, - ConstantValue(..) | ConstantRange(..) | IntRange(..) | NonExhaustive => vec![], + ConstantValue(..) | FloatRange(..) | IntRange(..) | NonExhaustive => vec![], } } @@ -830,7 +830,7 @@ impl<'tcx> Constructor<'tcx> { }, FixedLenSlice(length) => *length, VarLenSlice(prefix, suffix) => prefix + suffix, - ConstantValue(..) | ConstantRange(..) | IntRange(..) | NonExhaustive => 0, + ConstantValue(..) | FloatRange(..) | IntRange(..) | NonExhaustive => 0, } } @@ -894,10 +894,8 @@ impl<'tcx> Constructor<'tcx> { PatKind::Slice { prefix, slice: Some(wild), suffix } } &ConstantValue(value) => PatKind::Constant { value }, - &ConstantRange(lo, hi, end) => PatKind::Range(PatRange { lo, hi, end }), - IntRange(range) => { - return range.to_pat(cx.tcx); - } + &FloatRange(lo, hi, end) => PatKind::Range(PatRange { lo, hi, end }), + IntRange(range) => return range.to_pat(cx.tcx), NonExhaustive => PatKind::Wild, }; @@ -1297,7 +1295,7 @@ impl<'tcx> IntRange<'tcx> { let (lo, hi) = (lo ^ bias, hi ^ bias); let offset = (*end == RangeEnd::Excluded) as u128; if lo > hi || (lo == hi && *end == RangeEnd::Excluded) { - // This hould have been caught earlier by E0030 + // This should have been caught earlier by E0030. bug!("malformed range pattern: {}..={}", lo, (hi - offset)); } Some(IntRange { range: lo..=(hi - offset), ty, span }) @@ -1692,7 +1690,7 @@ fn pat_constructor<'tcx>( ) { Some(IntRange(int_range)) } else { - Some(ConstantRange(lo, hi, end)) + Some(FloatRange(lo, hi, end)) } } PatKind::Array { .. } => match pat.ty.kind { @@ -2091,7 +2089,7 @@ fn constructor_covered_by_range<'tcx>( }; let (ctor_from, ctor_to, ctor_end) = match *ctor { ConstantValue(value) => (value, value, RangeEnd::Included), - ConstantRange(from, to, ctor_end) => (from, to, ctor_end), + FloatRange(from, to, ctor_end) => (from, to, ctor_end), _ => bug!("`constructor_covered_by_range` called with {:?}", ctor), }; trace!("constructor_covered_by_range {:#?}, {:#?}, {:#?}, {}", ctor, pat_from, pat_to, ty);