Skip to content

Commit

Permalink
Use VarLenSlice consistently when splitting constructors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Nadrieril committed Nov 5, 2019
1 parent d582ed5 commit f774eb6
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 25 deletions.
23 changes: 19 additions & 4 deletions src/librustc_mir/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/pattern/usefulness/match-slice-patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

fn check(list: &[Option<()>]) {
match list {
//~^ ERROR `&[_, Some(_), None, _]` not covered
//~^ ERROR `&[_, Some(_), .., None, _]` not covered
&[] => {},
&[_] => {},
&[_, _] => {},
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/pattern/usefulness/match-slice-patterns.stderr
Original file line number Diff line number Diff line change
@@ -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

Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/pattern/usefulness/slice-patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn main() {
[.., false] => {}
}
match s {
//~^ ERROR `&[false, true]` not covered
//~^ ERROR `&[false, .., true]` not covered
[] => {}
[true, ..] => {}
[.., false] => {}
Expand All @@ -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] => {}
Expand All @@ -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
[] => {}
[_] => {}
[_, _] => {}
Expand Down
24 changes: 12 additions & 12 deletions src/test/ui/pattern/usefulness/slice-patterns.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down

0 comments on commit f774eb6

Please sign in to comment.