From 143fb4357f484c1c55811b97bace92482ba0ec5a Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 6 Nov 2019 16:52:37 +0000 Subject: [PATCH 01/11] Tweak VarLenSlice subtraction --- src/librustc_mir/hair/pattern/_match.rs | 67 +++++++++++++------------ 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index e30d6819d04f1..555772f8b58d3 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -655,44 +655,45 @@ impl<'tcx> Constructor<'tcx> { .into_iter() .flat_map(|pos_ctor| -> SmallVec<[Constructor<'tcx>; 1]> { // Compute `pos_ctor \ neg_ctor`. - match (&pos_ctor, neg_ctor) { - (&FixedLenSlice(pos_len), &VarLenSlice(neg_prefix, neg_suffix)) => { - let neg_len = neg_prefix + neg_suffix; - if neg_len <= pos_len { + match pos_ctor { + FixedLenSlice(pos_len) => match *neg_ctor { + FixedLenSlice(neg_len) if neg_len == pos_len => smallvec![], + VarLenSlice(neg_prefix, neg_suffix) + if neg_prefix + neg_suffix <= pos_len => + { smallvec![] - } else { - smallvec![pos_ctor] } - } - ( - &VarLenSlice(pos_prefix, pos_suffix), - &VarLenSlice(neg_prefix, neg_suffix), - ) => { - let neg_len = neg_prefix + neg_suffix; - let pos_len = pos_prefix + pos_suffix; - if neg_len <= pos_len { - smallvec![] - } else { - (pos_len..neg_len).map(FixedLenSlice).collect() - } - } - (&VarLenSlice(pos_prefix, pos_suffix), &FixedLenSlice(neg_len)) => { + _ => smallvec![pos_ctor], + }, + VarLenSlice(pos_prefix, pos_suffix) => { let pos_len = pos_prefix + pos_suffix; - if neg_len < pos_len { - smallvec![pos_ctor] - } else { - (pos_len..neg_len) - .map(FixedLenSlice) - // We know that `neg_len + 1 >= pos_len >= pos_suffix`. - .chain(Some(VarLenSlice( - neg_len + 1 - pos_suffix, - pos_suffix, - ))) - .collect() + match *neg_ctor { + FixedLenSlice(neg_len) if neg_len >= pos_len => { + (pos_len..neg_len) + .map(FixedLenSlice) + // We know that `neg_len + 1 >= pos_len >= + // pos_suffix`. + .chain(Some(VarLenSlice( + neg_len + 1 - pos_suffix, + pos_suffix, + ))) + .collect() + } + VarLenSlice(neg_prefix, neg_suffix) => { + let neg_len = neg_prefix + neg_suffix; + if neg_len <= pos_len { + smallvec![] + } else { + (pos_len..neg_len).map(FixedLenSlice).collect() + } + } + _ => smallvec![pos_ctor], } } - _ if pos_ctor == *neg_ctor => smallvec![], - _ => smallvec![pos_ctor], + _ => bug!( + "unexpected ctor while subtracting from VarLenSlice: {:?}", + pos_ctor + ), } }) .collect(); From c00ecfa8d268ad34c90667a249384618a19ea39b Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 16 Nov 2019 16:18:46 +0000 Subject: [PATCH 02/11] Add some tests --- .../slice-patterns-exhaustiveness.rs | 18 +++++-- .../slice-patterns-exhaustiveness.stderr | 54 ++++++++++++------- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs index 86cdf160618bf..11ca06e36adc0 100644 --- a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs +++ b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs @@ -5,6 +5,20 @@ fn main() { let s1: &[bool; 1] = &[false; 1]; let s2: &[bool; 2] = &[false; 2]; let s3: &[bool; 3] = &[false; 3]; + let s10: &[bool; 10] = &[false; 10]; + + match s2 { + //~^ ERROR `&[false, _]` not covered + [true, .., true] => {} + } + match s3 { + //~^ ERROR `&[false, _, _]` not covered + [true, .., true] => {} + } + match s10 { + //~^ ERROR `&[false, _, _, _, _, _, _, _, _, _]` not covered + [true, .., true] => {} + } match s1 { [true, ..] => {} @@ -27,10 +41,6 @@ fn main() { [.., false] => {} } - match s3 { - //~^ ERROR `&[false, _, _]` not covered - [true, .., true] => {} - } match s { //~^ ERROR `&[_, ..]` not covered [] => {} diff --git a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr index 1391b520556dc..7bf283ec741ba 100644 --- a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr +++ b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr @@ -1,5 +1,29 @@ +error[E0004]: non-exhaustive patterns: `&[false, _]` not covered + --> $DIR/slice-patterns-exhaustiveness.rs:10:11 + | +LL | match s2 { + | ^^ pattern `&[false, _]` not covered + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + +error[E0004]: non-exhaustive patterns: `&[false, _, _]` not covered + --> $DIR/slice-patterns-exhaustiveness.rs:14:11 + | +LL | match s3 { + | ^^ pattern `&[false, _, _]` not covered + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + +error[E0004]: non-exhaustive patterns: `&[false, _, _, _, _, _, _, _, _, _]` not covered + --> $DIR/slice-patterns-exhaustiveness.rs:18:11 + | +LL | match s10 { + | ^^^ pattern `&[false, _, _, _, _, _, _, _, _, _]` not covered + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + error[E0004]: non-exhaustive patterns: `&[false, true]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:13:11 + --> $DIR/slice-patterns-exhaustiveness.rs:27:11 | LL | match s2 { | ^^ pattern `&[false, true]` not covered @@ -7,7 +31,7 @@ LL | match s2 { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `&[false, _, true]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:18:11 + --> $DIR/slice-patterns-exhaustiveness.rs:32:11 | LL | match s3 { | ^^ pattern `&[false, _, true]` not covered @@ -15,23 +39,15 @@ LL | match s3 { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `&[false, .., true]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:23:11 + --> $DIR/slice-patterns-exhaustiveness.rs:37:11 | LL | match s { | ^ pattern `&[false, .., true]` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms -error[E0004]: non-exhaustive patterns: `&[false, _, _]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:30:11 - | -LL | match s3 { - | ^^ pattern `&[false, _, _]` not covered - | - = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms - error[E0004]: non-exhaustive patterns: `&[_, ..]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:34:11 + --> $DIR/slice-patterns-exhaustiveness.rs:44:11 | LL | match s { | ^ pattern `&[_, ..]` not covered @@ -39,7 +55,7 @@ LL | match s { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `&[_, _, ..]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:38:11 + --> $DIR/slice-patterns-exhaustiveness.rs:48:11 | LL | match s { | ^ pattern `&[_, _, ..]` not covered @@ -47,7 +63,7 @@ LL | match s { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `&[false, ..]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:43:11 + --> $DIR/slice-patterns-exhaustiveness.rs:53:11 | LL | match s { | ^ pattern `&[false, ..]` not covered @@ -55,7 +71,7 @@ LL | match s { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `&[false, _, ..]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:48:11 + --> $DIR/slice-patterns-exhaustiveness.rs:58:11 | LL | match s { | ^ pattern `&[false, _, ..]` not covered @@ -63,7 +79,7 @@ LL | match s { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `&[_, .., false]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:54:11 + --> $DIR/slice-patterns-exhaustiveness.rs:64:11 | LL | match s { | ^ pattern `&[_, .., false]` not covered @@ -71,7 +87,7 @@ LL | match s { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `&[_, _, .., true]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:61:11 + --> $DIR/slice-patterns-exhaustiveness.rs:71:11 | LL | match s { | ^ pattern `&[_, _, .., true]` not covered @@ -79,13 +95,13 @@ LL | match s { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms error[E0004]: non-exhaustive patterns: `&[true, _, .., _]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:68:11 + --> $DIR/slice-patterns-exhaustiveness.rs:78:11 | LL | match s { | ^ pattern `&[true, _, .., _]` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms -error: aborting due to 11 previous errors +error: aborting due to 13 previous errors For more information about this error, try `rustc --explain E0004`. From d93c1b320c1dc6d38165c70e5f1c13dba895c179 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 16 Nov 2019 16:05:32 +0000 Subject: [PATCH 03/11] Introduce new FixedLenSlice constructor It is used in the case where a variable-length slice pattern is used to match on an array of known size. This allows considering only those entries in the array that are captured by one of the patterns. As a side-effect, diagnostics improve a bit for those cases. --- src/librustc_mir/hair/pattern/_match.rs | 107 +++++++++++++----- .../match-byte-array-patterns-2.stderr | 4 +- .../slice-patterns-exhaustiveness.rs | 6 +- .../slice-patterns-exhaustiveness.stderr | 12 +- 4 files changed, 92 insertions(+), 37 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 555772f8b58d3..fdee2ffee983c 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -597,6 +597,14 @@ enum Constructor<'tcx> { FloatRange(&'tcx ty::Const<'tcx>, &'tcx ty::Const<'tcx>, RangeEnd), /// Array patterns of length `n`. FixedLenSlice(u64), + /// Array patterns of length `len`, but for which we only care about the `prefix` first values + /// and the `suffix` last values. This avoids unnecessarily going through values we know to be + /// uninteresting, which can be a major problem for large arrays. + LazyFixedLenSlice { + len: u64, // The actual length of the array + prefix: u64, + suffix: u64, + }, /// Slice patterns. Captures any array constructor of `length >= i + j`. VarLenSlice(u64, u64), /// Fake extra constructor for enums that aren't allowed to be matched exhaustively. @@ -606,7 +614,7 @@ enum Constructor<'tcx> { impl<'tcx> Constructor<'tcx> { fn is_slice(&self) -> bool { match self { - FixedLenSlice { .. } | VarLenSlice { .. } => true, + FixedLenSlice { .. } | LazyFixedLenSlice { .. } | VarLenSlice { .. } => true, _ => false, } } @@ -635,9 +643,9 @@ impl<'tcx> Constructor<'tcx> { Single | Variant(_) | ConstantValue(..) | FloatRange(..) => { if other_ctors.iter().any(|c| c == self) { vec![] } else { vec![self.clone()] } } - &FixedLenSlice(self_len) => { + &FixedLenSlice(self_len) | &LazyFixedLenSlice { len: self_len, .. } => { let overlaps = |c: &Constructor<'_>| match *c { - FixedLenSlice(other_len) => other_len == self_len, + FixedLenSlice(len) | LazyFixedLenSlice { len, .. } => len == self_len, VarLenSlice(prefix, suffix) => prefix + suffix <= self_len, _ => false, }; @@ -657,7 +665,12 @@ impl<'tcx> Constructor<'tcx> { // Compute `pos_ctor \ neg_ctor`. match pos_ctor { FixedLenSlice(pos_len) => match *neg_ctor { - FixedLenSlice(neg_len) if neg_len == pos_len => smallvec![], + FixedLenSlice(neg_len) + | LazyFixedLenSlice { len: neg_len, .. } + if neg_len == pos_len => + { + smallvec![] + } VarLenSlice(neg_prefix, neg_suffix) if neg_prefix + neg_suffix <= pos_len => { @@ -668,7 +681,10 @@ impl<'tcx> Constructor<'tcx> { VarLenSlice(pos_prefix, pos_suffix) => { let pos_len = pos_prefix + pos_suffix; match *neg_ctor { - FixedLenSlice(neg_len) if neg_len >= pos_len => { + FixedLenSlice(neg_len) + | LazyFixedLenSlice { len: neg_len, .. } + if neg_len >= pos_len => + { (pos_len..neg_len) .map(FixedLenSlice) // We know that `neg_len + 1 >= pos_len >= @@ -799,7 +815,7 @@ impl<'tcx> Constructor<'tcx> { } _ => vec![], }, - FixedLenSlice(_) | VarLenSlice(..) => match ty.kind { + FixedLenSlice(_) | LazyFixedLenSlice { .. } | VarLenSlice(..) => match ty.kind { ty::Slice(ty) | ty::Array(ty, _) => { let arity = self.arity(cx, ty); (0..arity).map(|_| Pat::wildcard_from_ty(ty)).collect() @@ -830,7 +846,9 @@ impl<'tcx> Constructor<'tcx> { _ => 0, }, FixedLenSlice(length) => *length, - VarLenSlice(prefix, suffix) => prefix + suffix, + VarLenSlice(prefix, suffix) | LazyFixedLenSlice { prefix, suffix, .. } => { + prefix + suffix + } ConstantValue(..) | FloatRange(..) | IntRange(..) | NonExhaustive => 0, } } @@ -888,8 +906,11 @@ impl<'tcx> Constructor<'tcx> { FixedLenSlice(_) => { PatKind::Slice { prefix: subpatterns.collect(), slice: None, suffix: vec![] } } - &VarLenSlice(prefix_len, _) => { - let prefix = subpatterns.by_ref().take(prefix_len as usize).collect(); + LazyFixedLenSlice { len, prefix, suffix } if prefix + suffix == *len => { + PatKind::Slice { prefix: subpatterns.collect(), slice: None, suffix: vec![] } + } + VarLenSlice(prefix, _) | LazyFixedLenSlice { prefix, .. } => { + let prefix = subpatterns.by_ref().take(*prefix as usize).collect(); let suffix = subpatterns.collect(); let wild = Pat::wildcard_from_ty(ty); PatKind::Slice { prefix, slice: Some(wild), suffix } @@ -1106,7 +1127,11 @@ fn all_constructors<'a, 'tcx>( } 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)] } + if len != 0 && cx.is_uninhabited(sub_ty) { + vec![] + } else { + vec![LazyFixedLenSlice { len, prefix: 0, suffix: 0 }] + } } // Treat arrays of a constant but unknown length like slices. ty::Array(ref sub_ty, _) | ty::Slice(ref sub_ty) => { @@ -1694,10 +1719,19 @@ fn pat_constructor<'tcx>( Some(FloatRange(lo, hi, end)) } } - 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), - }, + PatKind::Array { ref prefix, ref slice, ref suffix } => { + let len = match pat.ty.kind { + ty::Array(_, length) => length.eval_usize(tcx, param_env), + _ => span_bug!(pat.span, "bad ty {:?} for array pattern", pat.ty), + }; + let prefix = prefix.len() as u64; + let suffix = suffix.len() as u64; + if slice.is_some() { + Some(LazyFixedLenSlice { len, prefix, suffix }) + } else { + Some(FixedLenSlice(len)) + } + } PatKind::Slice { ref prefix, ref slice, ref suffix } => { let prefix = prefix.len() as u64; let suffix = suffix.len() as u64; @@ -1833,6 +1867,7 @@ fn split_grouped_constructors<'p, 'tcx>( ) -> Vec> { let ty = pcx.ty; let mut split_ctors = Vec::with_capacity(ctors.len()); + debug!("split_grouped_constructors({:#?}, {:#?})", matrix, ctors); for ctor in ctors.into_iter() { match ctor { @@ -1920,7 +1955,8 @@ fn split_grouped_constructors<'p, 'tcx>( .map(IntRange), ); } - VarLenSlice(self_prefix, self_suffix) => { + VarLenSlice(self_prefix, self_suffix) + | LazyFixedLenSlice { prefix: self_prefix, suffix: self_suffix, .. } => { // The exhaustiveness-checking paper does not include any details on // checking variable-length slice patterns. However, they are matched // by an infinite collection of fixed-length array patterns. @@ -2005,11 +2041,13 @@ fn split_grouped_constructors<'p, 'tcx>( _ => {} } } - PatKind::Slice { ref prefix, slice: None, ref suffix } => { + PatKind::Slice { ref prefix, slice: None, ref suffix } + | PatKind::Array { ref prefix, slice: None, ref suffix } => { let fixed_len = prefix.len() as u64 + suffix.len() as u64; max_fixed_len = cmp::max(max_fixed_len, fixed_len); } - PatKind::Slice { ref prefix, slice: Some(_), ref suffix } => { + PatKind::Slice { ref prefix, slice: Some(_), ref suffix } + | PatKind::Array { ref prefix, slice: Some(_), ref suffix } => { max_prefix_len = cmp::max(max_prefix_len, prefix.len() as u64); max_suffix_len = cmp::max(max_suffix_len, suffix.len() as u64); } @@ -2027,20 +2065,37 @@ fn split_grouped_constructors<'p, 'tcx>( max_prefix_len = max_fixed_len + 1 - max_suffix_len; } - // `ctor` originally covered the range `(self_prefix + self_suffix..infinity)`. We - // now split it into two: lengths smaller than `max_prefix_len + max_suffix_len` - // are treated independently as fixed-lengths slices, and lengths above are - // captured by a final VarLenSlice constructor. - split_ctors.extend( - (self_prefix + self_suffix..max_prefix_len + max_suffix_len).map(FixedLenSlice), - ); - split_ctors.push(VarLenSlice(max_prefix_len, max_suffix_len)); + match ctor { + LazyFixedLenSlice { len, .. } => { + if max_prefix_len + max_suffix_len < len { + split_ctors.push(LazyFixedLenSlice { + len, + prefix: max_prefix_len, + suffix: max_suffix_len, + }); + } else { + split_ctors.push(FixedLenSlice(len)); + } + } + _ => { + // `ctor` originally covered the range `(self_prefix + self_suffix..infinity)`. We + // now split it into two: lengths smaller than `max_prefix_len + max_suffix_len` + // are treated independently as fixed-lengths slices, and lengths above are + // captured by a final VarLenSlice constructor. + split_ctors.extend( + (self_prefix + self_suffix..max_prefix_len + max_suffix_len) + .map(FixedLenSlice), + ); + split_ctors.push(VarLenSlice(max_prefix_len, max_suffix_len)); + } + } } // Any other constructor can be used unchanged. _ => split_ctors.push(ctor), } } + debug!("split_grouped_constructors(..)={:#?}", split_ctors); split_ctors } @@ -2252,7 +2307,7 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( PatKind::Array { ref prefix, ref slice, ref suffix } | PatKind::Slice { ref prefix, ref slice, ref suffix } => match *constructor { - FixedLenSlice(..) | VarLenSlice(..) => { + FixedLenSlice(..) | LazyFixedLenSlice { .. } | VarLenSlice(..) => { let pat_len = prefix.len() + suffix.len(); if let Some(slice_count) = ctor_wild_subpatterns.len().checked_sub(pat_len) { if slice_count == 0 || slice.is_some() { diff --git a/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr b/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr index 6e52072e3bfec..63ed49094fc50 100644 --- a/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr +++ b/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr @@ -1,8 +1,8 @@ -error[E0004]: non-exhaustive patterns: `&[_, _, _, _]` not covered +error[E0004]: non-exhaustive patterns: `&[..]` not covered --> $DIR/match-byte-array-patterns-2.rs:4:11 | LL | match buf { - | ^^^ pattern `&[_, _, _, _]` not covered + | ^^^ pattern `&[..]` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms diff --git a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs index 11ca06e36adc0..5881c35d356b9 100644 --- a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs +++ b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs @@ -12,11 +12,11 @@ fn main() { [true, .., true] => {} } match s3 { - //~^ ERROR `&[false, _, _]` not covered + //~^ ERROR `&[false, .., _]` not covered [true, .., true] => {} } match s10 { - //~^ ERROR `&[false, _, _, _, _, _, _, _, _, _]` not covered + //~^ ERROR `&[false, .., _]` not covered [true, .., true] => {} } @@ -30,7 +30,7 @@ fn main() { [.., false] => {} } match s3 { - //~^ ERROR `&[false, _, true]` not covered + //~^ ERROR `&[false, .., true]` not covered [true, ..] => {} [.., false] => {} } diff --git a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr index 7bf283ec741ba..9a711f2a441dc 100644 --- a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr +++ b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr @@ -6,19 +6,19 @@ LL | match s2 { | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms -error[E0004]: non-exhaustive patterns: `&[false, _, _]` not covered +error[E0004]: non-exhaustive patterns: `&[false, .., _]` not covered --> $DIR/slice-patterns-exhaustiveness.rs:14:11 | LL | match s3 { - | ^^ pattern `&[false, _, _]` not covered + | ^^ pattern `&[false, .., _]` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms -error[E0004]: non-exhaustive patterns: `&[false, _, _, _, _, _, _, _, _, _]` not covered +error[E0004]: non-exhaustive patterns: `&[false, .., _]` not covered --> $DIR/slice-patterns-exhaustiveness.rs:18:11 | LL | match s10 { - | ^^^ pattern `&[false, _, _, _, _, _, _, _, _, _]` not covered + | ^^^ pattern `&[false, .., _]` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms @@ -30,11 +30,11 @@ LL | match s2 { | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms -error[E0004]: non-exhaustive patterns: `&[false, _, true]` not covered +error[E0004]: non-exhaustive patterns: `&[false, .., true]` not covered --> $DIR/slice-patterns-exhaustiveness.rs:32:11 | LL | match s3 { - | ^^ pattern `&[false, _, true]` not covered + | ^^ pattern `&[false, .., true]` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms From 5b420a9dc476d23e54a0b3733cad173866a1fb1d Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 16 Nov 2019 16:43:55 +0000 Subject: [PATCH 04/11] Add regression test for issue 53820 --- .../pattern/issue-53820-slice-pattern-large-array.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 src/test/ui/pattern/issue-53820-slice-pattern-large-array.rs diff --git a/src/test/ui/pattern/issue-53820-slice-pattern-large-array.rs b/src/test/ui/pattern/issue-53820-slice-pattern-large-array.rs new file mode 100644 index 0000000000000..38c830e4d69bb --- /dev/null +++ b/src/test/ui/pattern/issue-53820-slice-pattern-large-array.rs @@ -0,0 +1,10 @@ +// check-pass +#![feature(slice_patterns)] + +fn main() { + const LARGE_SIZE: usize = 1024 * 1024; + let [..] = [0u8; LARGE_SIZE]; + match [0u8; LARGE_SIZE] { + [..] => {} + } +} From d0cfef32a8439b6936d9348e2ec17397fe2baeb2 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 17 Nov 2019 15:54:44 +0000 Subject: [PATCH 05/11] Unify the various slice constructors --- src/librustc_mir/hair/pattern/_match.rs | 181 +++++++++++++----------- 1 file changed, 99 insertions(+), 82 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index fdee2ffee983c..2ccddabfb2f41 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -225,6 +225,7 @@ /// 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::SliceKind::*; use self::Usefulness::*; use self::WitnessPreference::*; @@ -582,6 +583,23 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { } } +#[derive(Copy, Clone, Debug, PartialEq)] +enum SliceKind { + /// Array patterns of length `n`. + FixedLen(u64), + /// Slice patterns. Captures any array constructor of `length >= i + j`. + VarLen(u64, u64), +} + +impl SliceKind { + fn arity(self) -> u64 { + match self { + FixedLen(length) => length, + VarLen(prefix, suffix) => prefix + suffix, + } + } +} + #[derive(Clone, Debug, PartialEq)] enum Constructor<'tcx> { /// The constructor of all patterns that don't vary by constructor, @@ -595,18 +613,12 @@ enum Constructor<'tcx> { IntRange(IntRange<'tcx>), /// 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), - /// Array patterns of length `len`, but for which we only care about the `prefix` first values - /// and the `suffix` last values. This avoids unnecessarily going through values we know to be - /// uninteresting, which can be a major problem for large arrays. - LazyFixedLenSlice { - len: u64, // The actual length of the array - prefix: u64, - suffix: u64, + /// Array and slice patterns. + Slice { + // The length of the type of the pattern, if fixed. + type_len: Option, + kind: SliceKind, }, - /// Slice patterns. Captures any array constructor of `length >= i + j`. - VarLenSlice(u64, u64), /// Fake extra constructor for enums that aren't allowed to be matched exhaustively. NonExhaustive, } @@ -614,7 +626,7 @@ enum Constructor<'tcx> { impl<'tcx> Constructor<'tcx> { fn is_slice(&self) -> bool { match self { - FixedLenSlice { .. } | LazyFixedLenSlice { .. } | VarLenSlice { .. } => true, + Slice { .. } => true, _ => false, } } @@ -643,20 +655,24 @@ impl<'tcx> Constructor<'tcx> { Single | Variant(_) | ConstantValue(..) | FloatRange(..) => { if other_ctors.iter().any(|c| c == self) { vec![] } else { vec![self.clone()] } } - &FixedLenSlice(self_len) | &LazyFixedLenSlice { len: self_len, .. } => { + &Slice { type_len: Some(self_len), .. } | &Slice { kind: FixedLen(self_len), .. } => { let overlaps = |c: &Constructor<'_>| match *c { - FixedLenSlice(len) | LazyFixedLenSlice { len, .. } => len == self_len, - VarLenSlice(prefix, suffix) => prefix + suffix <= self_len, + Slice { type_len: Some(len), .. } | Slice { kind: FixedLen(len), .. } => { + len == self_len + } + Slice { type_len: None, kind: VarLen(prefix, suffix) } => { + prefix + suffix <= self_len + } _ => false, }; if other_ctors.iter().any(overlaps) { vec![] } else { vec![self.clone()] } } - VarLenSlice(..) => { + Slice { type_len: None, kind: VarLen(..) } => { let mut remaining_ctors = vec![self.clone()]; // For each used ctor, subtract from the current set of constructors. // Naming: we remove the "neg" constructors from the "pos" ones. - // Remember, `VarLenSlice(i, j)` covers the union of `FixedLenSlice` from + // Remember, `VarLen(i, j)` covers the union of `FixedLen` from // `i + j` to infinity. for neg_ctor in other_ctors { remaining_ctors = remaining_ctors @@ -664,50 +680,64 @@ impl<'tcx> Constructor<'tcx> { .flat_map(|pos_ctor| -> SmallVec<[Constructor<'tcx>; 1]> { // Compute `pos_ctor \ neg_ctor`. match pos_ctor { - FixedLenSlice(pos_len) => match *neg_ctor { - FixedLenSlice(neg_len) - | LazyFixedLenSlice { len: neg_len, .. } + Slice { type_len: Some(pos_len), .. } + | Slice { kind: FixedLen(pos_len), .. } => match *neg_ctor { + Slice { type_len: Some(neg_len), .. } + | Slice { kind: FixedLen(neg_len), .. } if neg_len == pos_len => { smallvec![] } - VarLenSlice(neg_prefix, neg_suffix) - if neg_prefix + neg_suffix <= pos_len => - { - smallvec![] - } + Slice { + type_len: None, + kind: VarLen(neg_prefix, neg_suffix), + } if neg_prefix + neg_suffix <= pos_len => smallvec![], _ => smallvec![pos_ctor], }, - VarLenSlice(pos_prefix, pos_suffix) => { + Slice { type_len: None, kind: VarLen(pos_prefix, pos_suffix) } => { let pos_len = pos_prefix + pos_suffix; match *neg_ctor { - FixedLenSlice(neg_len) - | LazyFixedLenSlice { len: neg_len, .. } + Slice { type_len: Some(neg_len), .. } + | Slice { kind: FixedLen(neg_len), .. } if neg_len >= pos_len => { (pos_len..neg_len) - .map(FixedLenSlice) + .map(|l| Slice { + type_len: None, + kind: FixedLen(l), + }) // We know that `neg_len + 1 >= pos_len >= // pos_suffix`. - .chain(Some(VarLenSlice( - neg_len + 1 - pos_suffix, - pos_suffix, - ))) + .chain(Some(Slice { + type_len: None, + kind: VarLen( + neg_len + 1 - pos_suffix, + pos_suffix, + ), + })) .collect() } - VarLenSlice(neg_prefix, neg_suffix) => { + Slice { + type_len: None, + kind: VarLen(neg_prefix, neg_suffix), + } => { let neg_len = neg_prefix + neg_suffix; if neg_len <= pos_len { smallvec![] } else { - (pos_len..neg_len).map(FixedLenSlice).collect() + (pos_len..neg_len) + .map(|l| Slice { + type_len: None, + kind: FixedLen(l), + }) + .collect() } } _ => smallvec![pos_ctor], } } _ => bug!( - "unexpected ctor while subtracting from VarLenSlice: {:?}", + "unexpected ctor while subtracting from VarLen: {:?}", pos_ctor ), } @@ -815,7 +845,7 @@ impl<'tcx> Constructor<'tcx> { } _ => vec![], }, - FixedLenSlice(_) | LazyFixedLenSlice { .. } | VarLenSlice(..) => match ty.kind { + Slice { .. } => match ty.kind { ty::Slice(ty) | ty::Array(ty, _) => { let arity = self.arity(cx, ty); (0..arity).map(|_| Pat::wildcard_from_ty(ty)).collect() @@ -845,10 +875,7 @@ impl<'tcx> Constructor<'tcx> { } _ => 0, }, - FixedLenSlice(length) => *length, - VarLenSlice(prefix, suffix) | LazyFixedLenSlice { prefix, suffix, .. } => { - prefix + suffix - } + Slice { kind, .. } => kind.arity(), ConstantValue(..) | FloatRange(..) | IntRange(..) | NonExhaustive => 0, } } @@ -903,13 +930,15 @@ impl<'tcx> Constructor<'tcx> { ty::Slice(_) | ty::Array(..) => bug!("bad slice pattern {:?} {:?}", self, ty), _ => PatKind::Wild, }, - FixedLenSlice(_) => { + Slice { kind: FixedLen(_), .. } => { PatKind::Slice { prefix: subpatterns.collect(), slice: None, suffix: vec![] } } - LazyFixedLenSlice { len, prefix, suffix } if prefix + suffix == *len => { + Slice { type_len: Some(len), kind: VarLen(prefix, suffix) } + if prefix + suffix == *len => + { PatKind::Slice { prefix: subpatterns.collect(), slice: None, suffix: vec![] } } - VarLenSlice(prefix, _) | LazyFixedLenSlice { prefix, .. } => { + Slice { kind: VarLen(prefix, _), .. } => { let prefix = subpatterns.by_ref().take(*prefix as usize).collect(); let suffix = subpatterns.collect(); let wild = Pat::wildcard_from_ty(ty); @@ -1130,15 +1159,15 @@ fn all_constructors<'a, 'tcx>( if len != 0 && cx.is_uninhabited(sub_ty) { vec![] } else { - vec![LazyFixedLenSlice { len, prefix: 0, suffix: 0 }] + vec![Slice { type_len: Some(len), kind: VarLen(0, 0) }] } } // Treat arrays of a constant but unknown length like slices. ty::Array(ref sub_ty, _) | ty::Slice(ref sub_ty) => { if cx.is_uninhabited(sub_ty) { - vec![FixedLenSlice(0)] + vec![Slice { type_len: None, kind: FixedLen(0) }] } else { - vec![VarLenSlice(0, 0)] + vec![Slice { type_len: None, kind: VarLen(0, 0) }] } } ty::Adt(def, substs) if def.is_enum() => { @@ -1719,27 +1748,18 @@ fn pat_constructor<'tcx>( Some(FloatRange(lo, hi, end)) } } - PatKind::Array { ref prefix, ref slice, ref suffix } => { - let len = match pat.ty.kind { - ty::Array(_, length) => length.eval_usize(tcx, param_env), - _ => span_bug!(pat.span, "bad ty {:?} for array pattern", pat.ty), + PatKind::Array { ref prefix, ref slice, ref suffix } + | PatKind::Slice { ref prefix, ref slice, ref suffix } => { + let type_len = match pat.ty.kind { + ty::Array(_, length) => Some(length.eval_usize(tcx, param_env)), + ty::Slice(_) => None, + _ => span_bug!(pat.span, "bad ty {:?} for slice pattern", pat.ty), }; let prefix = prefix.len() as u64; let suffix = suffix.len() as u64; - if slice.is_some() { - Some(LazyFixedLenSlice { len, prefix, suffix }) - } else { - Some(FixedLenSlice(len)) - } - } - PatKind::Slice { ref prefix, ref slice, ref suffix } => { - let prefix = prefix.len() as u64; - let suffix = suffix.len() as u64; - if slice.is_some() { - Some(VarLenSlice(prefix, suffix)) - } else { - Some(FixedLenSlice(prefix + suffix)) - } + let kind = + if slice.is_some() { VarLen(prefix, suffix) } else { FixedLen(prefix + suffix) }; + Some(Slice { type_len, kind }) } PatKind::Or { .. } => { bug!("support for or-patterns has not been fully implemented yet."); @@ -1955,8 +1975,7 @@ fn split_grouped_constructors<'p, 'tcx>( .map(IntRange), ); } - VarLenSlice(self_prefix, self_suffix) - | LazyFixedLenSlice { prefix: self_prefix, suffix: self_suffix, .. } => { + Slice { kind: VarLen(self_prefix, self_suffix), type_len } => { // The exhaustiveness-checking paper does not include any details on // checking variable-length slice patterns. However, they are matched // by an infinite collection of fixed-length array patterns. @@ -2065,28 +2084,26 @@ fn split_grouped_constructors<'p, 'tcx>( max_prefix_len = max_fixed_len + 1 - max_suffix_len; } - match ctor { - LazyFixedLenSlice { len, .. } => { - if max_prefix_len + max_suffix_len < len { - split_ctors.push(LazyFixedLenSlice { - len, - prefix: max_prefix_len, - suffix: max_suffix_len, - }); + match type_len { + Some(len) => { + let kind = if max_prefix_len + max_suffix_len < len { + VarLen(max_prefix_len, max_suffix_len) } else { - split_ctors.push(FixedLenSlice(len)); - } + FixedLen(len) + }; + split_ctors.push(Slice { type_len, kind }); } - _ => { + None => { // `ctor` originally covered the range `(self_prefix + self_suffix..infinity)`. We // now split it into two: lengths smaller than `max_prefix_len + max_suffix_len` // are treated independently as fixed-lengths slices, and lengths above are - // captured by a final VarLenSlice constructor. + // captured by a final VarLen constructor. split_ctors.extend( (self_prefix + self_suffix..max_prefix_len + max_suffix_len) - .map(FixedLenSlice), + .map(|len| Slice { type_len, kind: FixedLen(len) }), ); - split_ctors.push(VarLenSlice(max_prefix_len, max_suffix_len)); + split_ctors + .push(Slice { type_len, kind: VarLen(max_prefix_len, max_suffix_len) }); } } } @@ -2307,7 +2324,7 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( PatKind::Array { ref prefix, ref slice, ref suffix } | PatKind::Slice { ref prefix, ref slice, ref suffix } => match *constructor { - FixedLenSlice(..) | LazyFixedLenSlice { .. } | VarLenSlice(..) => { + Slice { .. } => { let pat_len = prefix.len() + suffix.len(); if let Some(slice_count) = ctor_wild_subpatterns.len().checked_sub(pat_len) { if slice_count == 0 || slice.is_some() { From 54e97e889b97aa04d477329e26299a1bbc6fefe2 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 17 Nov 2019 17:33:39 +0000 Subject: [PATCH 06/11] Factor out slice constructor struct and simplify --- src/librustc_mir/hair/pattern/_match.rs | 301 +++++++++++++----------- 1 file changed, 160 insertions(+), 141 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 2ccddabfb2f41..51ee1f29bf8e7 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -583,21 +583,110 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { } } -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] enum SliceKind { - /// Array patterns of length `n`. + /// Patterns of length `n` (`[x, y]`). FixedLen(u64), - /// Slice patterns. Captures any array constructor of `length >= i + j`. + /// Patterns using the `..` notation (`[x, .., y]`). Captures any array constructor of `length + /// >= i + j`. In the case where `array_len` is `Some(_)`, this indicates that we only care + /// about the first `i` and the last `j` values of the array, and everything in between is a + /// wildcard `_`. VarLen(u64, u64), } -impl SliceKind { - fn arity(self) -> u64 { +/// A constructor for array and slice patterns. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +struct Slice { + /// `None` if the matched value is a slice, `Some(n)` if it is an array of size `n`. + array_len: Option, + /// The kind of pattern it is: fixed-length `[x, y]` or variable length `[x, .., y]`. + kind: SliceKind, +} + +impl Slice { + /// Returns what patterns this constructor covers: either fixed-length patterns or + /// variable-length patterns. + fn pattern_kind(self) -> SliceKind { + match self { + Slice { array_len: Some(len), kind: VarLen(prefix, suffix) } + if prefix + suffix == len => + { + FixedLen(len) + } + _ => self.kind, + } + } + + /// Returns what values this constructor covers: either values of only one given length, or + /// values of length above a given length. + /// This is different from `pattern_kind()` because in some cases the pattern only takes into + /// account a subset of the entries of the array, but still only captures values of a given + /// length. + fn value_kind(self) -> SliceKind { match self { + Slice { array_len: Some(len), kind: VarLen(_, _) } => FixedLen(len), + _ => self.kind, + } + } + + fn arity(self) -> u64 { + match self.pattern_kind() { FixedLen(length) => length, VarLen(prefix, suffix) => prefix + suffix, } } + + /// Whether this pattern includes patterns of length `other_len`. + fn covers_length(self, other_len: u64) -> bool { + match self.value_kind() { + FixedLen(len) => len == other_len, + VarLen(prefix, suffix) => prefix + suffix <= other_len, + } + } + + /// Returns a collection of slices that spans the values covered by `self`, subtracted by the + /// values covered by `other`: i.e., `self \ other` (in set notation). + fn subtract(self, other: Self) -> SmallVec<[Self; 1]> { + // Remember, `VarLen(i, j)` covers the union of `FixedLen` from `i + j` to infinity. + // Naming: we remove the "neg" constructors from the "pos" ones. + match self.value_kind() { + FixedLen(pos_len) => { + if other.covers_length(pos_len) { + smallvec![] + } else { + smallvec![self] + } + } + VarLen(pos_prefix, pos_suffix) => { + let pos_len = pos_prefix + pos_suffix; + match other.value_kind() { + FixedLen(neg_len) => { + if neg_len < pos_len { + smallvec![self] + } else { + (pos_len..neg_len) + .map(FixedLen) + // We know that `neg_len + 1 >= pos_len >= pos_suffix`. + .chain(Some(VarLen(neg_len + 1 - pos_suffix, pos_suffix))) + .map(|kind| Slice { array_len: None, kind }) + .collect() + } + } + VarLen(neg_prefix, neg_suffix) => { + let neg_len = neg_prefix + neg_suffix; + if neg_len <= pos_len { + smallvec![] + } else { + (pos_len..neg_len) + .map(FixedLen) + .map(|kind| Slice { array_len: None, kind }) + .collect() + } + } + } + } + } + } } #[derive(Clone, Debug, PartialEq)] @@ -614,11 +703,7 @@ enum Constructor<'tcx> { /// Ranges of floating-point literal values (`2.0..=5.2`). FloatRange(&'tcx ty::Const<'tcx>, &'tcx ty::Const<'tcx>, RangeEnd), /// Array and slice patterns. - Slice { - // The length of the type of the pattern, if fixed. - type_len: Option, - kind: SliceKind, - }, + Slice(Slice), /// Fake extra constructor for enums that aren't allowed to be matched exhaustively. NonExhaustive, } @@ -626,7 +711,7 @@ enum Constructor<'tcx> { impl<'tcx> Constructor<'tcx> { fn is_slice(&self) -> bool { match self { - Slice { .. } => true, + Slice(_) => true, _ => false, } } @@ -655,104 +740,41 @@ impl<'tcx> Constructor<'tcx> { Single | Variant(_) | ConstantValue(..) | FloatRange(..) => { if other_ctors.iter().any(|c| c == self) { vec![] } else { vec![self.clone()] } } - &Slice { type_len: Some(self_len), .. } | &Slice { kind: FixedLen(self_len), .. } => { - let overlaps = |c: &Constructor<'_>| match *c { - Slice { type_len: Some(len), .. } | Slice { kind: FixedLen(len), .. } => { - len == self_len - } - Slice { type_len: None, kind: VarLen(prefix, suffix) } => { - prefix + suffix <= self_len + &Slice(slice) => match slice.value_kind() { + FixedLen(self_len) => { + let overlaps = |c: &Constructor<'_>| match *c { + Slice(other_slice) => other_slice.covers_length(self_len), + _ => false, + }; + if other_ctors.iter().any(overlaps) { vec![] } else { vec![Slice(slice)] } + } + VarLen(..) => { + let mut remaining_slices = vec![slice]; + + // For each used slice, subtract from the current set of slices. + // Naming: we remove the "neg" constructors from the "pos" ones. + for neg_ctor in other_ctors { + let neg_slice = match neg_ctor { + Slice(slice) => *slice, + // FIXME(#65413): If `neg_ctor` is not a slice, we assume it doesn't + // cover any value here. + _ => continue, + }; + remaining_slices = remaining_slices + .into_iter() + .flat_map(|pos_slice| pos_slice.subtract(neg_slice)) + .collect(); + + // If the constructors that have been considered so far already cover + // the entire range of `self`, no need to look at more constructors. + if remaining_slices.is_empty() { + break; + } } - _ => false, - }; - if other_ctors.iter().any(overlaps) { vec![] } else { vec![self.clone()] } - } - Slice { type_len: None, kind: VarLen(..) } => { - let mut remaining_ctors = vec![self.clone()]; - - // For each used ctor, subtract from the current set of constructors. - // Naming: we remove the "neg" constructors from the "pos" ones. - // Remember, `VarLen(i, j)` covers the union of `FixedLen` from - // `i + j` to infinity. - for neg_ctor in other_ctors { - remaining_ctors = remaining_ctors - .into_iter() - .flat_map(|pos_ctor| -> SmallVec<[Constructor<'tcx>; 1]> { - // Compute `pos_ctor \ neg_ctor`. - match pos_ctor { - Slice { type_len: Some(pos_len), .. } - | Slice { kind: FixedLen(pos_len), .. } => match *neg_ctor { - Slice { type_len: Some(neg_len), .. } - | Slice { kind: FixedLen(neg_len), .. } - if neg_len == pos_len => - { - smallvec![] - } - Slice { - type_len: None, - kind: VarLen(neg_prefix, neg_suffix), - } if neg_prefix + neg_suffix <= pos_len => smallvec![], - _ => smallvec![pos_ctor], - }, - Slice { type_len: None, kind: VarLen(pos_prefix, pos_suffix) } => { - let pos_len = pos_prefix + pos_suffix; - match *neg_ctor { - Slice { type_len: Some(neg_len), .. } - | Slice { kind: FixedLen(neg_len), .. } - if neg_len >= pos_len => - { - (pos_len..neg_len) - .map(|l| Slice { - type_len: None, - kind: FixedLen(l), - }) - // We know that `neg_len + 1 >= pos_len >= - // pos_suffix`. - .chain(Some(Slice { - type_len: None, - kind: VarLen( - neg_len + 1 - pos_suffix, - pos_suffix, - ), - })) - .collect() - } - Slice { - type_len: None, - kind: VarLen(neg_prefix, neg_suffix), - } => { - let neg_len = neg_prefix + neg_suffix; - if neg_len <= pos_len { - smallvec![] - } else { - (pos_len..neg_len) - .map(|l| Slice { - type_len: None, - kind: FixedLen(l), - }) - .collect() - } - } - _ => smallvec![pos_ctor], - } - } - _ => bug!( - "unexpected ctor while subtracting from VarLen: {:?}", - pos_ctor - ), - } - }) - .collect(); - // If the constructors that have been considered so far already cover - // the entire range of `self`, no need to look at more constructors. - if remaining_ctors.is_empty() { - break; - } + remaining_slices.into_iter().map(Slice).collect() } - - remaining_ctors - } + }, IntRange(self_range) => { let mut remaining_ranges = vec![self_range.clone()]; for other_ctor in other_ctors { @@ -845,7 +867,7 @@ impl<'tcx> Constructor<'tcx> { } _ => vec![], }, - Slice { .. } => match ty.kind { + Slice(_) => match ty.kind { ty::Slice(ty) | ty::Array(ty, _) => { let arity = self.arity(cx, ty); (0..arity).map(|_| Pat::wildcard_from_ty(ty)).collect() @@ -875,7 +897,7 @@ impl<'tcx> Constructor<'tcx> { } _ => 0, }, - Slice { kind, .. } => kind.arity(), + Slice(slice) => slice.arity(), ConstantValue(..) | FloatRange(..) | IntRange(..) | NonExhaustive => 0, } } @@ -930,20 +952,17 @@ impl<'tcx> Constructor<'tcx> { ty::Slice(_) | ty::Array(..) => bug!("bad slice pattern {:?} {:?}", self, ty), _ => PatKind::Wild, }, - Slice { kind: FixedLen(_), .. } => { - PatKind::Slice { prefix: subpatterns.collect(), slice: None, suffix: vec![] } - } - Slice { type_len: Some(len), kind: VarLen(prefix, suffix) } - if prefix + suffix == *len => - { - PatKind::Slice { prefix: subpatterns.collect(), slice: None, suffix: vec![] } - } - Slice { kind: VarLen(prefix, _), .. } => { - let prefix = subpatterns.by_ref().take(*prefix as usize).collect(); - let suffix = subpatterns.collect(); - let wild = Pat::wildcard_from_ty(ty); - PatKind::Slice { prefix, slice: Some(wild), suffix } - } + Slice(slice) => match slice.pattern_kind() { + FixedLen(_) => { + PatKind::Slice { prefix: subpatterns.collect(), slice: None, suffix: vec![] } + } + VarLen(prefix, _) => { + let prefix = subpatterns.by_ref().take(prefix as usize).collect(); + let suffix = subpatterns.collect(); + let wild = Pat::wildcard_from_ty(ty); + PatKind::Slice { prefix, slice: Some(wild), suffix } + } + }, &ConstantValue(value) => PatKind::Constant { value }, &FloatRange(lo, hi, end) => PatKind::Range(PatRange { lo, hi, end }), IntRange(range) => return range.to_pat(cx.tcx), @@ -1159,16 +1178,13 @@ fn all_constructors<'a, 'tcx>( if len != 0 && cx.is_uninhabited(sub_ty) { vec![] } else { - vec![Slice { type_len: Some(len), kind: VarLen(0, 0) }] + vec![Slice(Slice { array_len: Some(len), kind: VarLen(0, 0) })] } } // Treat arrays of a constant but unknown length like slices. ty::Array(ref sub_ty, _) | ty::Slice(ref sub_ty) => { - if cx.is_uninhabited(sub_ty) { - vec![Slice { type_len: None, kind: FixedLen(0) }] - } else { - vec![Slice { type_len: None, kind: VarLen(0, 0) }] - } + let kind = if cx.is_uninhabited(sub_ty) { FixedLen(0) } else { VarLen(0, 0) }; + vec![Slice(Slice { array_len: None, kind })] } ty::Adt(def, substs) if def.is_enum() => { let ctors: Vec<_> = def @@ -1750,7 +1766,7 @@ fn pat_constructor<'tcx>( } PatKind::Array { ref prefix, ref slice, ref suffix } | PatKind::Slice { ref prefix, ref slice, ref suffix } => { - let type_len = match pat.ty.kind { + let array_len = match pat.ty.kind { ty::Array(_, length) => Some(length.eval_usize(tcx, param_env)), ty::Slice(_) => None, _ => span_bug!(pat.span, "bad ty {:?} for slice pattern", pat.ty), @@ -1759,7 +1775,7 @@ fn pat_constructor<'tcx>( let suffix = suffix.len() as u64; let kind = if slice.is_some() { VarLen(prefix, suffix) } else { FixedLen(prefix + suffix) }; - Some(Slice { type_len, kind }) + Some(Slice(Slice { array_len, kind })) } PatKind::Or { .. } => { bug!("support for or-patterns has not been fully implemented yet."); @@ -1975,7 +1991,7 @@ fn split_grouped_constructors<'p, 'tcx>( .map(IntRange), ); } - Slice { kind: VarLen(self_prefix, self_suffix), type_len } => { + Slice(Slice { array_len, kind: VarLen(self_prefix, self_suffix) }) => { // The exhaustiveness-checking paper does not include any details on // checking variable-length slice patterns. However, they are matched // by an infinite collection of fixed-length array patterns. @@ -2084,26 +2100,29 @@ fn split_grouped_constructors<'p, 'tcx>( max_prefix_len = max_fixed_len + 1 - max_suffix_len; } - match type_len { + match array_len { Some(len) => { let kind = if max_prefix_len + max_suffix_len < len { VarLen(max_prefix_len, max_suffix_len) } else { FixedLen(len) }; - split_ctors.push(Slice { type_len, kind }); + split_ctors.push(Slice(Slice { array_len, kind })); } None => { - // `ctor` originally covered the range `(self_prefix + self_suffix..infinity)`. We - // now split it into two: lengths smaller than `max_prefix_len + max_suffix_len` - // are treated independently as fixed-lengths slices, and lengths above are - // captured by a final VarLen constructor. + // `ctor` originally covered the range `(self_prefix + + // self_suffix..infinity)`. We now split it into two: lengths smaller than + // `max_prefix_len + max_suffix_len` are treated independently as + // fixed-lengths slices, and lengths above are captured by a final VarLen + // constructor. split_ctors.extend( (self_prefix + self_suffix..max_prefix_len + max_suffix_len) - .map(|len| Slice { type_len, kind: FixedLen(len) }), + .map(|len| Slice(Slice { array_len, kind: FixedLen(len) })), ); - split_ctors - .push(Slice { type_len, kind: VarLen(max_prefix_len, max_suffix_len) }); + split_ctors.push(Slice(Slice { + array_len, + kind: VarLen(max_prefix_len, max_suffix_len), + })); } } } @@ -2324,7 +2343,7 @@ fn specialize_one_pattern<'p, 'a: 'p, 'q: 'p, 'tcx>( PatKind::Array { ref prefix, ref slice, ref suffix } | PatKind::Slice { ref prefix, ref slice, ref suffix } => match *constructor { - Slice { .. } => { + Slice(_) => { let pat_len = prefix.len() + suffix.len(); if let Some(slice_count) = ctor_wild_subpatterns.len().checked_sub(pat_len) { if slice_count == 0 || slice.is_some() { From daa117eab715f38d08facfb826b04116bc9fe0b7 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 17 Nov 2019 17:48:45 +0000 Subject: [PATCH 07/11] Small improvement to exhaustiveness diagnostics --- src/librustc_mir/hair/pattern/_match.rs | 16 ++++++++++++++-- .../usefulness/slice-patterns-exhaustiveness.rs | 4 ++-- .../slice-patterns-exhaustiveness.stderr | 8 ++++---- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 51ee1f29bf8e7..d379f606da4e1 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -957,8 +957,20 @@ impl<'tcx> Constructor<'tcx> { PatKind::Slice { prefix: subpatterns.collect(), slice: None, suffix: vec![] } } VarLen(prefix, _) => { - let prefix = subpatterns.by_ref().take(prefix as usize).collect(); - let suffix = subpatterns.collect(); + let mut prefix: Vec<_> = subpatterns.by_ref().take(prefix as usize).collect(); + let mut suffix: Vec<_> = subpatterns.collect(); + if slice.array_len.is_some() { + // Improves diagnostics a bit: if the type is a known-size array, instead + // of reporting `[x, _, .., _, y]`, we prefer to report `[x, .., y]`. + // This is incorrect if the size is not known, since `[_, ..]` captures + // arrays of lengths `>= 1` whereas `[..]` captures any length. + while !suffix.is_empty() && suffix.first().unwrap().is_wildcard() { + suffix.remove(0); + } + while !prefix.is_empty() && prefix.last().unwrap().is_wildcard() { + prefix.pop(); + } + } let wild = Pat::wildcard_from_ty(ty); PatKind::Slice { prefix, slice: Some(wild), suffix } } diff --git a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs index 5881c35d356b9..eb3dfac950f79 100644 --- a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs +++ b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs @@ -12,11 +12,11 @@ fn main() { [true, .., true] => {} } match s3 { - //~^ ERROR `&[false, .., _]` not covered + //~^ ERROR `&[false, ..]` not covered [true, .., true] => {} } match s10 { - //~^ ERROR `&[false, .., _]` not covered + //~^ ERROR `&[false, ..]` not covered [true, .., true] => {} } diff --git a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr index 9a711f2a441dc..ebadedccfea21 100644 --- a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr +++ b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr @@ -6,19 +6,19 @@ LL | match s2 { | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms -error[E0004]: non-exhaustive patterns: `&[false, .., _]` not covered +error[E0004]: non-exhaustive patterns: `&[false, ..]` not covered --> $DIR/slice-patterns-exhaustiveness.rs:14:11 | LL | match s3 { - | ^^ pattern `&[false, .., _]` not covered + | ^^ pattern `&[false, ..]` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms -error[E0004]: non-exhaustive patterns: `&[false, .., _]` not covered +error[E0004]: non-exhaustive patterns: `&[false, ..]` not covered --> $DIR/slice-patterns-exhaustiveness.rs:18:11 | LL | match s10 { - | ^^^ pattern `&[false, .., _]` not covered + | ^^^ pattern `&[false, ..]` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms From bd0e3ccd0bf81b57f115b344dcf424a3ed626cab Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 17 Nov 2019 19:08:01 +0000 Subject: [PATCH 08/11] Store SliceKinds directly when subtracting --- src/librustc_mir/hair/pattern/_match.rs | 111 +++++++++++++----------- 1 file changed, 58 insertions(+), 53 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index d379f606da4e1..182c9e22e4188 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -594,43 +594,9 @@ enum SliceKind { VarLen(u64, u64), } -/// A constructor for array and slice patterns. -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -struct Slice { - /// `None` if the matched value is a slice, `Some(n)` if it is an array of size `n`. - array_len: Option, - /// The kind of pattern it is: fixed-length `[x, y]` or variable length `[x, .., y]`. - kind: SliceKind, -} - -impl Slice { - /// Returns what patterns this constructor covers: either fixed-length patterns or - /// variable-length patterns. - fn pattern_kind(self) -> SliceKind { - match self { - Slice { array_len: Some(len), kind: VarLen(prefix, suffix) } - if prefix + suffix == len => - { - FixedLen(len) - } - _ => self.kind, - } - } - - /// Returns what values this constructor covers: either values of only one given length, or - /// values of length above a given length. - /// This is different from `pattern_kind()` because in some cases the pattern only takes into - /// account a subset of the entries of the array, but still only captures values of a given - /// length. - fn value_kind(self) -> SliceKind { - match self { - Slice { array_len: Some(len), kind: VarLen(_, _) } => FixedLen(len), - _ => self.kind, - } - } - +impl SliceKind { fn arity(self) -> u64 { - match self.pattern_kind() { + match self { FixedLen(length) => length, VarLen(prefix, suffix) => prefix + suffix, } @@ -638,7 +604,7 @@ impl Slice { /// Whether this pattern includes patterns of length `other_len`. fn covers_length(self, other_len: u64) -> bool { - match self.value_kind() { + match self { FixedLen(len) => len == other_len, VarLen(prefix, suffix) => prefix + suffix <= other_len, } @@ -649,7 +615,7 @@ impl Slice { fn subtract(self, other: Self) -> SmallVec<[Self; 1]> { // Remember, `VarLen(i, j)` covers the union of `FixedLen` from `i + j` to infinity. // Naming: we remove the "neg" constructors from the "pos" ones. - match self.value_kind() { + match self { FixedLen(pos_len) => { if other.covers_length(pos_len) { smallvec![] @@ -659,7 +625,7 @@ impl Slice { } VarLen(pos_prefix, pos_suffix) => { let pos_len = pos_prefix + pos_suffix; - match other.value_kind() { + match other { FixedLen(neg_len) => { if neg_len < pos_len { smallvec![self] @@ -668,7 +634,6 @@ impl Slice { .map(FixedLen) // We know that `neg_len + 1 >= pos_len >= pos_suffix`. .chain(Some(VarLen(neg_len + 1 - pos_suffix, pos_suffix))) - .map(|kind| Slice { array_len: None, kind }) .collect() } } @@ -677,10 +642,7 @@ impl Slice { if neg_len <= pos_len { smallvec![] } else { - (pos_len..neg_len) - .map(FixedLen) - .map(|kind| Slice { array_len: None, kind }) - .collect() + (pos_len..neg_len).map(FixedLen).collect() } } } @@ -689,6 +651,46 @@ impl Slice { } } +/// A constructor for array and slice patterns. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +struct Slice { + /// `None` if the matched value is a slice, `Some(n)` if it is an array of size `n`. + array_len: Option, + /// The kind of pattern it is: fixed-length `[x, y]` or variable length `[x, .., y]`. + kind: SliceKind, +} + +impl Slice { + /// Returns what patterns this constructor covers: either fixed-length patterns or + /// variable-length patterns. + fn pattern_kind(self) -> SliceKind { + match self { + Slice { array_len: Some(len), kind: VarLen(prefix, suffix) } + if prefix + suffix == len => + { + FixedLen(len) + } + _ => self.kind, + } + } + + /// Returns what values this constructor covers: either values of only one given length, or + /// values of length above a given length. + /// This is different from `pattern_kind()` because in some cases the pattern only takes into + /// account a subset of the entries of the array, but still only captures values of a given + /// length. + fn value_kind(self) -> SliceKind { + match self { + Slice { array_len: Some(len), kind: VarLen(_, _) } => FixedLen(len), + _ => self.kind, + } + } + + fn arity(self) -> u64 { + self.pattern_kind().arity() + } +} + #[derive(Clone, Debug, PartialEq)] enum Constructor<'tcx> { /// The constructor of all patterns that don't vary by constructor, @@ -743,26 +745,25 @@ impl<'tcx> Constructor<'tcx> { &Slice(slice) => match slice.value_kind() { FixedLen(self_len) => { let overlaps = |c: &Constructor<'_>| match *c { - Slice(other_slice) => other_slice.covers_length(self_len), + Slice(other_slice) => other_slice.value_kind().covers_length(self_len), _ => false, }; if other_ctors.iter().any(overlaps) { vec![] } else { vec![Slice(slice)] } } VarLen(..) => { - let mut remaining_slices = vec![slice]; + let mut remaining_slices = vec![slice.value_kind()]; // For each used slice, subtract from the current set of slices. - // Naming: we remove the "neg" constructors from the "pos" ones. - for neg_ctor in other_ctors { - let neg_slice = match neg_ctor { - Slice(slice) => *slice, - // FIXME(#65413): If `neg_ctor` is not a slice, we assume it doesn't + for other_ctor in other_ctors { + let other_slice = match other_ctor { + Slice(slice) => slice.value_kind(), + // FIXME(#65413): If `other_ctor` is not a slice, we assume it doesn't // cover any value here. _ => continue, }; remaining_slices = remaining_slices .into_iter() - .flat_map(|pos_slice| pos_slice.subtract(neg_slice)) + .flat_map(|remaining_slice| remaining_slice.subtract(other_slice)) .collect(); // If the constructors that have been considered so far already cover @@ -772,7 +773,11 @@ impl<'tcx> Constructor<'tcx> { } } - remaining_slices.into_iter().map(Slice).collect() + remaining_slices + .into_iter() + .map(|kind| Slice { array_len: slice.array_len, kind }) + .map(Slice) + .collect() } }, IntRange(self_range) => { From 7f5e0441e2246451e3b03ac907c4308d071e99fd Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 17 Nov 2019 19:24:48 +0000 Subject: [PATCH 09/11] `ConstantValue` is the only other ctor allowed when subtracting from slice ctors --- src/librustc_mir/hair/pattern/_match.rs | 72 +++++++++++++------------ 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 182c9e22e4188..a6afea228818c 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -742,44 +742,50 @@ impl<'tcx> Constructor<'tcx> { Single | Variant(_) | ConstantValue(..) | FloatRange(..) => { if other_ctors.iter().any(|c| c == self) { vec![] } else { vec![self.clone()] } } - &Slice(slice) => match slice.value_kind() { - FixedLen(self_len) => { - let overlaps = |c: &Constructor<'_>| match *c { - Slice(other_slice) => other_slice.value_kind().covers_length(self_len), - _ => false, - }; - if other_ctors.iter().any(overlaps) { vec![] } else { vec![Slice(slice)] } - } - VarLen(..) => { - let mut remaining_slices = vec![slice.value_kind()]; - - // For each used slice, subtract from the current set of slices. - for other_ctor in other_ctors { - let other_slice = match other_ctor { - Slice(slice) => slice.value_kind(), - // FIXME(#65413): If `other_ctor` is not a slice, we assume it doesn't - // cover any value here. - _ => continue, - }; - remaining_slices = remaining_slices - .into_iter() - .flat_map(|remaining_slice| remaining_slice.subtract(other_slice)) - .collect(); + &Slice(slice) => { + let mut other_slices = other_ctors + .iter() + .filter_map(|c: &Constructor<'_>| match c { + Slice(slice) => Some(*slice), + // FIXME(#65413): We ignore `ConstantValue`s here. + ConstantValue(..) => None, + _ => bug!("bad slice pattern constructor {:?}", c), + }) + .map(Slice::value_kind); - // If the constructors that have been considered so far already cover - // the entire range of `self`, no need to look at more constructors. - if remaining_slices.is_empty() { - break; + match slice.value_kind() { + FixedLen(self_len) => { + if other_slices.any(|other_slice| other_slice.covers_length(self_len)) { + vec![] + } else { + vec![Slice(slice)] } } + kind @ VarLen(..) => { + let mut remaining_slices = vec![kind]; + + // For each used slice, subtract from the current set of slices. + for other_slice in other_slices { + remaining_slices = remaining_slices + .into_iter() + .flat_map(|remaining_slice| remaining_slice.subtract(other_slice)) + .collect(); + + // If the constructors that have been considered so far already cover + // the entire range of `self`, no need to look at more constructors. + if remaining_slices.is_empty() { + break; + } + } - remaining_slices - .into_iter() - .map(|kind| Slice { array_len: slice.array_len, kind }) - .map(Slice) - .collect() + remaining_slices + .into_iter() + .map(|kind| Slice { array_len: slice.array_len, kind }) + .map(Slice) + .collect() + } } - }, + } IntRange(self_range) => { let mut remaining_ranges = vec![self_range.clone()]; for other_ctor in other_ctors { From 2079ae3a52db9c3f2022892b8473e6eb1e0f1656 Mon Sep 17 00:00:00 2001 From: Nadrieril Feneanar Date: Mon, 18 Nov 2019 15:40:49 +0000 Subject: [PATCH 10/11] Update src/test/ui/pattern/issue-53820-slice-pattern-large-array.rs Co-Authored-By: Mazdak Farrokhzad --- src/test/ui/pattern/issue-53820-slice-pattern-large-array.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/ui/pattern/issue-53820-slice-pattern-large-array.rs b/src/test/ui/pattern/issue-53820-slice-pattern-large-array.rs index 38c830e4d69bb..c910cded96be4 100644 --- a/src/test/ui/pattern/issue-53820-slice-pattern-large-array.rs +++ b/src/test/ui/pattern/issue-53820-slice-pattern-large-array.rs @@ -1,4 +1,7 @@ // check-pass + +// This used to cause a stack overflow in the compiler. + #![feature(slice_patterns)] fn main() { From 1425ae1154f3541a32e9ca607c09ce50cfb1298e Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 18 Nov 2019 15:47:51 +0000 Subject: [PATCH 11/11] Tweak diagnostics code --- 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 a6afea228818c..5e7a7f01e7a32 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -969,19 +969,21 @@ impl<'tcx> Constructor<'tcx> { } VarLen(prefix, _) => { let mut prefix: Vec<_> = subpatterns.by_ref().take(prefix as usize).collect(); - let mut suffix: Vec<_> = subpatterns.collect(); if slice.array_len.is_some() { // Improves diagnostics a bit: if the type is a known-size array, instead // of reporting `[x, _, .., _, y]`, we prefer to report `[x, .., y]`. // This is incorrect if the size is not known, since `[_, ..]` captures // arrays of lengths `>= 1` whereas `[..]` captures any length. - while !suffix.is_empty() && suffix.first().unwrap().is_wildcard() { - suffix.remove(0); - } while !prefix.is_empty() && prefix.last().unwrap().is_wildcard() { prefix.pop(); } } + let suffix: Vec<_> = if slice.array_len.is_some() { + // Same as above. + subpatterns.skip_while(Pat::is_wildcard).collect() + } else { + subpatterns.collect() + }; let wild = Pat::wildcard_from_ty(ty); PatKind::Slice { prefix, slice: Some(wild), suffix } }