From f774eb69fd19cabaa17dfe91dd0a1b23a2dce842 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 5 Nov 2019 16:30:04 +0000 Subject: [PATCH] Use VarLenSlice consistently when splitting constructors The previous behaviour ignored slice lengths above a certain length because it could not do otherwise. We now have VarLenSlice however, that can represent the ignored lengths to make the algorithm more consistent. This does not change the correctness of the algorithm, but makes it easier to reason about. As a nice side-effect, exhaustiveness errors have improved: they now capture all missing lengths instead of only the shortest. --- src/librustc_mir/hair/pattern/_match.rs | 23 ++++++++++++++---- .../usefulness/match-slice-patterns.rs | 2 +- .../usefulness/match-slice-patterns.stderr | 4 ++-- .../ui/pattern/usefulness/slice-patterns.rs | 12 +++++----- .../pattern/usefulness/slice-patterns.stderr | 24 +++++++++---------- 5 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 15f88334e8d5..8d7e605f7c9a 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -2042,7 +2042,7 @@ fn split_grouped_constructors<'p, 'tcx>( // // Of course, if fixed-length patterns exist, we must be sure // that our length is large enough to miss them all, so - // we can pick `L = max(FIXED_LEN+1 ∪ {max(PREFIX_LEN) + max(SUFFIX_LEN)})` + // we can pick `L = max(max(FIXED_LEN)+1, max(PREFIX_LEN) + max(SUFFIX_LEN))` // // for example, with the above pair of patterns, all elements // but the first and last can be added/removed, so any @@ -2080,9 +2080,24 @@ fn split_grouped_constructors<'p, 'tcx>( } } - let max_slice_length = cmp::max(max_fixed_len + 1, max_prefix_len + max_suffix_len); - split_ctors - .extend((self_prefix + self_suffix..=max_slice_length).map(FixedLenSlice)) + // For diagnostics, we keep the prefix and suffix lengths separate, so in the case + // where `max_fixed_len+1` is the largest, we adapt `max_prefix_len` accordingly, + // so that `L = max_prefix_len + max_suffix_len`. + if max_fixed_len + 1 >= max_prefix_len + max_suffix_len { + // The subtraction can't overflow thanks to the above check. + // The new `max_prefix_len` is also guaranteed to be larger than its previous + // value. + 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)); } // Any other constructor can be used unchanged. _ => split_ctors.push(ctor), diff --git a/src/test/ui/pattern/usefulness/match-slice-patterns.rs b/src/test/ui/pattern/usefulness/match-slice-patterns.rs index afbeb61e4415..af7fd53a1f1e 100644 --- a/src/test/ui/pattern/usefulness/match-slice-patterns.rs +++ b/src/test/ui/pattern/usefulness/match-slice-patterns.rs @@ -2,7 +2,7 @@ fn check(list: &[Option<()>]) { match list { - //~^ ERROR `&[_, Some(_), None, _]` not covered + //~^ ERROR `&[_, Some(_), .., None, _]` not covered &[] => {}, &[_] => {}, &[_, _] => {}, diff --git a/src/test/ui/pattern/usefulness/match-slice-patterns.stderr b/src/test/ui/pattern/usefulness/match-slice-patterns.stderr index 24769db34c93..72ae5d5fe3b3 100644 --- a/src/test/ui/pattern/usefulness/match-slice-patterns.stderr +++ b/src/test/ui/pattern/usefulness/match-slice-patterns.stderr @@ -1,8 +1,8 @@ -error[E0004]: non-exhaustive patterns: `&[_, Some(_), None, _]` not covered +error[E0004]: non-exhaustive patterns: `&[_, Some(_), .., None, _]` not covered --> $DIR/match-slice-patterns.rs:4:11 | LL | match list { - | ^^^^ pattern `&[_, Some(_), None, _]` not covered + | ^^^^ pattern `&[_, Some(_), .., None, _]` 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.rs b/src/test/ui/pattern/usefulness/slice-patterns.rs index e11f11ba7524..da2d40caf1a4 100644 --- a/src/test/ui/pattern/usefulness/slice-patterns.rs +++ b/src/test/ui/pattern/usefulness/slice-patterns.rs @@ -37,7 +37,7 @@ fn main() { [.., false] => {} } match s { - //~^ ERROR `&[false, true]` not covered + //~^ ERROR `&[false, .., true]` not covered [] => {} [true, ..] => {} [.., false] => {} @@ -57,18 +57,18 @@ fn main() { [_] => {} } match s { - //~^ ERROR `&[false]` not covered + //~^ ERROR `&[false, ..]` not covered [] => {} [true, ..] => {} } match s { - //~^ ERROR `&[false, _]` not covered + //~^ ERROR `&[false, _, ..]` not covered [] => {} [_] => {} [true, ..] => {} } match s { - //~^ ERROR `&[_, false]` not covered + //~^ ERROR `&[_, .., false]` not covered [] => {} [_] => {} [.., true] => {} @@ -94,14 +94,14 @@ fn main() { [..] => {} } match s { - //~^ ERROR `&[_, _, true]` not covered + //~^ ERROR `&[_, _, .., true]` not covered [] => {} [_] => {} [_, _] => {} [.., false] => {} } match s { - //~^ ERROR `&[true, _, _]` not covered + //~^ ERROR `&[true, _, .., _]` not covered [] => {} [_] => {} [_, _] => {} diff --git a/src/test/ui/pattern/usefulness/slice-patterns.stderr b/src/test/ui/pattern/usefulness/slice-patterns.stderr index 666e16627c57..6afe4705b0e6 100644 --- a/src/test/ui/pattern/usefulness/slice-patterns.stderr +++ b/src/test/ui/pattern/usefulness/slice-patterns.stderr @@ -14,11 +14,11 @@ 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 +error[E0004]: non-exhaustive patterns: `&[false, .., true]` not covered --> $DIR/slice-patterns.rs:39:11 | LL | match s { - | ^ 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 @@ -46,27 +46,27 @@ 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 +error[E0004]: non-exhaustive patterns: `&[false, ..]` not covered --> $DIR/slice-patterns.rs:59:11 | LL | match s { - | ^ 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.rs:64:11 | LL | match s { - | ^ 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.rs:70:11 | LL | match s { - | ^ 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 @@ -112,19 +112,19 @@ error: unreachable pattern LL | [false, true] => {} | ^^^^^^^^^^^^^ -error[E0004]: non-exhaustive patterns: `&[_, _, true]` not covered +error[E0004]: non-exhaustive patterns: `&[_, _, .., true]` not covered --> $DIR/slice-patterns.rs:96:11 | LL | match s { - | ^ pattern `&[_, _, true]` not covered + | ^ pattern `&[_, _, .., 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: `&[true, _, _]` not covered +error[E0004]: non-exhaustive patterns: `&[true, _, .., _]` not covered --> $DIR/slice-patterns.rs:103:11 | LL | match s { - | ^ pattern `&[true, _, _]` not covered + | ^ pattern `&[true, _, .., _]` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms