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 1 commit
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
58 changes: 50 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,52 @@ fn all_constructors(_cx: &mut MatchCheckCtxt, pcx: PatternContext) -> Vec<Constr
}
}

fn max_slice_length<'a: 'b, 'b, 'tcx, I>(
_cx: &mut MatchCheckCtxt<'a, 'tcx>,
patterns: I) -> usize
where I: Iterator<Item=&'b [&'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.
//
// To check that infinite set, we notice that for every length
// larger than the length of the maximum fixed-length pattern,
// only variable-length patterns apply.
//
// For variable length patterns, all elements after the first
// `prefix_len` but before the last `suffix_len` are matched by
// the wildcard "middle" pattern, and therefore can be added/removed
// without affecting the match result.
//
// This means that all patterns with length at least
// `max(max_fixed+1,max_prefix+max_suffix)` are equivalent, so we
// only need to check patterns from that length and below.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good comment!

A minor note: I think it would be marginally improved by a code example, showing what (e.g.) max_fixed etc mean in Rust code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I am a bit confused -- why is it max_fixed_len + 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It must be strictly larger than max_fixed, and >= max_fixed + max_suffix.


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

for row in patterns {
match *row[0].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 +499,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).chain(Some(v)))
};

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() {}