From d3ddb85eb1de721373b96d86a1a2dbf33dda3497 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 5 Nov 2016 13:32:35 +0200 Subject: [PATCH 1/2] _match: correct max_slice_length logic The logic used to be wildly wrong, but before the HAIR patch its wrongness was hidden by another bug. Fixes #37598. --- src/librustc_const_eval/_match.rs | 58 ++++++++++++++++--- src/test/compile-fail/match-slice-patterns.rs | 24 ++++++++ src/test/run-pass/issue-37598.rs | 21 +++++++ 3 files changed, 95 insertions(+), 8 deletions(-) create mode 100644 src/test/compile-fail/match-slice-patterns.rs create mode 100644 src/test/run-pass/issue-37598.rs diff --git a/src/librustc_const_eval/_match.rs b/src/librustc_const_eval/_match.rs index 7f5eb31612cb3..e7d3af87bdaa7 100644 --- a/src/librustc_const_eval/_match.rs +++ b/src/librustc_const_eval/_match.rs @@ -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}; @@ -419,6 +419,52 @@ fn all_constructors(_cx: &mut MatchCheckCtxt, pcx: PatternContext) -> Vec( + _cx: &mut MatchCheckCtxt<'a, 'tcx>, + patterns: I) -> usize + where I: Iterator]> +{ + // 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. + + 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 @@ -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]); diff --git a/src/test/compile-fail/match-slice-patterns.rs b/src/test/compile-fail/match-slice-patterns.rs new file mode 100644 index 0000000000000..c0fc75f9713a8 --- /dev/null +++ b/src/test/compile-fail/match-slice-patterns.rs @@ -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 or the MIT license +// , 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() {} diff --git a/src/test/run-pass/issue-37598.rs b/src/test/run-pass/issue-37598.rs new file mode 100644 index 0000000000000..d32d2fc295440 --- /dev/null +++ b/src/test/run-pass/issue-37598.rs @@ -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 or the MIT license +// , 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() {} From 1dad4b6bb5dcdcda8060d0a662824a50a9f22e82 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 8 Nov 2016 22:55:57 +0200 Subject: [PATCH 2/2] add more comment --- src/librustc_const_eval/_match.rs | 75 +++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 14 deletions(-) diff --git a/src/librustc_const_eval/_match.rs b/src/librustc_const_eval/_match.rs index e7d3af87bdaa7..6f01e9b1487d5 100644 --- a/src/librustc_const_eval/_match.rs +++ b/src/librustc_const_eval/_match.rs @@ -419,34 +419,81 @@ fn all_constructors(_cx: &mut MatchCheckCtxt, pcx: PatternContext) -> Vec( +fn max_slice_length<'a, 'tcx, I>( _cx: &mut MatchCheckCtxt<'a, 'tcx>, patterns: I) -> usize - where I: Iterator]> + where I: Iterator> { // 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. + // 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: // - // 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. + // 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`. // - // 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. + // 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[0].kind { + match *row.kind { PatternKind::Constant { value: ConstVal::ByteStr(ref data) } => { max_fixed_len = cmp::max(max_fixed_len, data.len()); } @@ -504,7 +551,7 @@ pub fn is_useful<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>, let pcx = PatternContext { ty: rows.iter().map(|r| r[0].ty).find(|ty| !ty.references_error()) .unwrap_or(v[0].ty), - max_slice_length: max_slice_length(cx, rows.iter().map(|r| &**r).chain(Some(v))) + 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]);