Skip to content

_match: correct max_slice_length logic #37603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 97 additions & 8 deletions src/librustc_const_eval/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use syntax_pos::{Span, DUMMY_SP};

use arena::TypedArena;

use std::cmp::Ordering;
use std::cmp::{self, Ordering};
use std::fmt;
use std::iter::{FromIterator, IntoIterator, repeat};

Expand Down Expand Up @@ -419,6 +419,99 @@ fn all_constructors(_cx: &mut MatchCheckCtxt, pcx: PatternContext) -> Vec<Constr
}
}

fn max_slice_length<'a, 'tcx, I>(
_cx: &mut MatchCheckCtxt<'a, 'tcx>,
patterns: I) -> usize
where I: Iterator<Item=&'a Pattern<'tcx>>
{
// 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.
//
// Checking the infinite set directly would take an infinite amount
// of time. However, it turns out that for each finite set of
// patterns `P`, all sufficiently large array lengths are equivalent:
//
// Each slice `s` with a "sufficiently-large" length `l ≥ L` that applies
// to exactly the subset `Pₜ` of `P` can be transformed to a slice
// `sₘ` for each sufficiently-large length `m` that applies to exactly
// the same subset of `P`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is now even more awesome =) But I admit I am kind of getting lost a bit. It would have been helpful (for me) to note that these are byte slices, and that -- effectively -- b"123" is equivalent to &[b'1', b'2', b'3'].

And then maybe some examples like:

match foo { // foo : &[u8]
    b"123" => ..., // matches byte strings of length 3
    b"1234" => ..., // matches byte strings of length 4
    &[b'1', ..c, b'9'] => , // matches byte strings of length 2..infinity
} // here `max_slice_check` yields 5, as `max_fixed_len` is 4, `max_prefix_len` is 1, `max_suffix_len` is 1
match foo { // foo : &[u8]
    b"123" => ..., // matches byte strings of length 3
    b"1234" => ..., // matches byte strings of length 4
    &[b'1', b'2',  ..c, b'7', b'8, b'9'] => , // matches byte strings of length 5..infinity
} // here `max_slice_check` again yields 5, as `max_fixed_len` is 4, `max_prefix_len` is 2, `max_suffix_len` is 3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not byte slices, in general (see examples with boolean slices and Option<()> slices).

If there is a byte literal, then as usual it is treated like the expanded byte slice.

//
// Because of that, each witness for reachability-checking from one
// of the sufficiently-large lengths can be transformed to an
// equally-valid witness from any other length, so we only have
// to check slice lengths from the "minimal sufficiently-large length"
// and below.
//
// Note that the fact that there is a *single* `sₘ` for each `m`
// not depending on the specific pattern in `P` is important: if
// you look at the pair of patterns
// `[true, ..]`
// `[.., false]`
// Then any slice of length ≥1 that matches one of these two
// patterns can be be trivially turned to a slice of any
// other length ≥1 that matches them and vice-versa - for
// but the slice from length 2 `[false, true]` that matches neither
// of these patterns can't be turned to a slice from length 1 that
// matches neither of these patterns, so we have to consider
// slices from length 2 there.
//
// Now, to see that that length exists and find it, observe that slice
// patterns are either "fixed-length" patterns (`[_, _, _]`) or
// "variable-length" patterns (`[_, .., _]`).
//
// For fixed-length patterns, all slices with lengths *longer* than
// the pattern's length have the same outcome (of not matching), so
// as long as `L` is greater than the pattern's length we can pick
// any `sₘ` from that length and get the same result.
//
// For variable-length patterns, the situation is more complicated,
// because as seen above the precise value of `sₘ` matters.
//
// However, for each variable-length pattern `p` with a prefix of length
// `plₚ` and suffix of length `slₚ`, only the first `plₚ` and the last
// `slₚ` elements are examined.
//
// Therefore, as long as `L` is positive (to avoid concerns about empty
// types), all elements after the maximum prefix length and before
// the maximum suffix length are not examined by any variable-length
// pattern, and therefore can be added/removed without affecting
// them - creating equivalent patterns from any sufficiently-large
// length.
//
// 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)})`
//
// for example, with the above pair of patterns, all elements
// but the first and last can be added/removed, so any
// witness of length ≥2 (say, `[false, false, true]`) can be
// turned to a witness from any other length ≥2.

let mut max_prefix_len = 0;
let mut max_suffix_len = 0;
let mut max_fixed_len = 0;

for row in patterns {
match *row.kind {
PatternKind::Constant { value: ConstVal::ByteStr(ref data) } => {
max_fixed_len = cmp::max(max_fixed_len, data.len());
}
PatternKind::Slice { ref prefix, slice: None, ref suffix } => {
let fixed_len = prefix.len() + suffix.len();
max_fixed_len = cmp::max(max_fixed_len, fixed_len);
}
PatternKind::Slice { ref prefix, slice: Some(_), ref suffix } => {
max_prefix_len = cmp::max(max_prefix_len, prefix.len());
max_suffix_len = cmp::max(max_suffix_len, suffix.len());
}
_ => {}
}
}

cmp::max(max_fixed_len + 1, max_prefix_len + max_suffix_len)
}

/// Algorithm from http://moscova.inria.fr/~maranget/papers/warn/index.html
///
/// Whether a vector `v` of patterns is 'useful' in relation to a set of such
Expand Down Expand Up @@ -453,16 +546,12 @@ pub fn is_useful<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>,

let &Matrix(ref rows) = matrix;
assert!(rows.iter().all(|r| r.len() == v.len()));


let pcx = PatternContext {
ty: rows.iter().map(|r| r[0].ty).find(|ty| !ty.references_error())
.unwrap_or(v[0].ty),
max_slice_length: rows.iter().filter_map(|row| match *row[0].kind {
PatternKind::Slice { ref prefix, slice: _, ref suffix } =>
Some(prefix.len() + suffix.len()),
PatternKind::Constant { value: ConstVal::ByteStr(ref data) } =>
Some(data.len()),
_ => None
}).max().map_or(0, |v| v + 1)
max_slice_length: max_slice_length(cx, rows.iter().map(|r| r[0]).chain(Some(v[0])))
};

debug!("is_useful_expand_first_col: pcx={:?}, expanding {:?}", pcx, v[0]);
Expand Down
24 changes: 24 additions & 0 deletions src/test/compile-fail/match-slice-patterns.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(advanced_slice_patterns, slice_patterns)]

fn check(list: &[Option<()>]) {
match list {
//~^ ERROR `&[None, Some(_), None, _]` and `&[Some(_), Some(_), None, _]` not covered
&[] => {},
&[_] => {},
&[_, _] => {},
&[_, None, ..] => {},
&[.., Some(_), _] => {},
}
}

fn main() {}
21 changes: 21 additions & 0 deletions src/test/run-pass/issue-37598.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(advanced_slice_patterns, slice_patterns)]

fn check(list: &[u8]) {
match list {
&[] => {},
&[_u1, _u2, ref _next..] => {},
&[_u1] => {},
}
}

fn main() {}