From 9ba9cd5fd5df75d12225429f5831fb968672c79d Mon Sep 17 00:00:00 2001 From: Andrew Cann Date: Thu, 1 Dec 2016 11:37:03 +0800 Subject: [PATCH] Improve error message, fix and add tests. Changes the non-exhaustive match error message to generate more general witnesses. --- src/librustc_const_eval/_match.rs | 69 +++++++++++-------- src/test/compile-fail/match-slice-patterns.rs | 2 +- .../compile-fail/uninhabited-irrefutable.rs | 38 ++++++++++ src/test/compile-fail/uninhabited-patterns.rs | 49 +++++++++++++ src/test/ui/check_match/issue-35609.stderr | 4 +- 5 files changed, 131 insertions(+), 31 deletions(-) create mode 100644 src/test/compile-fail/uninhabited-irrefutable.rs create mode 100644 src/test/compile-fail/uninhabited-patterns.rs diff --git a/src/librustc_const_eval/_match.rs b/src/librustc_const_eval/_match.rs index 1577e87049cc..b6d1d22015e0 100644 --- a/src/librustc_const_eval/_match.rs +++ b/src/librustc_const_eval/_match.rs @@ -359,25 +359,6 @@ impl<'tcx> Witness<'tcx> { } } -/// Return the set of constructors from the same type as the first column of `matrix`, -/// that are matched only by wildcard patterns from that first column. -/// -/// Therefore, if there is some pattern that is unmatched by `matrix`, it will -/// still be unmatched if the first constructor is replaced by any of the constructors -/// in the return value. -fn missing_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, - matrix: &Matrix, - pcx: PatternContext<'tcx>) -> Vec { - let used_constructors: Vec = - matrix.0.iter() - .flat_map(|row| pat_constructors(cx, row[0], pcx).unwrap_or(vec![])) - .collect(); - debug!("used_constructors = {:?}", used_constructors); - all_constructors(cx, pcx).into_iter() - .filter(|c| !used_constructors.contains(c)) - .collect() -} - /// This determines the set of all possible constructors of a pattern matching /// values of type `left_ty`. For vectors, this would normally be an infinite set /// @@ -586,10 +567,28 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, ).find(|result| result.is_useful()).unwrap_or(NotUseful) } else { debug!("is_useful - expanding wildcard"); - let constructors = missing_constructors(cx, matrix, pcx); - debug!("is_useful - missing_constructors = {:?}", constructors); - if constructors.is_empty() { - all_constructors(cx, pcx).into_iter().map(|c| { + + let used_ctors: Vec = rows.iter().flat_map(|row| { + pat_constructors(cx, row[0], pcx).unwrap_or(vec![]) + }).collect(); + debug!("used_ctors = {:?}", used_ctors); + let all_ctors = all_constructors(cx, pcx); + debug!("all_ctors = {:?}", all_ctors); + let missing_ctors: Vec = all_ctors.iter().filter(|c| { + !used_ctors.contains(*c) + }).cloned().collect(); + debug!("missing_ctors = {:?}", missing_ctors); + + // `missing_ctors` is the set of constructors from the same type as the + // first column of `matrix` that are matched only by wildcard patterns + // from the first column. + // + // Therefore, if there is some pattern that is unmatched by `matrix`, + // it will still be unmatched if the first constructor is replaced by + // any of the constructors in `missing_ctors` + + if missing_ctors.is_empty() { + all_ctors.into_iter().map(|c| { is_useful_specialized(cx, matrix, v, c.clone(), pcx.ty, witness) }).find(|result| result.is_useful()).unwrap_or(NotUseful) } else { @@ -603,11 +602,25 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, match is_useful(cx, &matrix, &v[1..], witness) { UsefulWithWitness(pats) => { let cx = &*cx; - UsefulWithWitness(pats.into_iter().flat_map(|witness| { - constructors.iter().map(move |ctor| { - witness.clone().push_wild_constructor(cx, ctor, pcx.ty) - }) - }).collect()) + let new_witnesses = if used_ctors.is_empty() { + // All constructors are unused. Add wild patterns + // rather than each individual constructor + pats.into_iter().map(|mut witness| { + witness.0.push(P(hir::Pat { + id: DUMMY_NODE_ID, + node: PatKind::Wild, + span: DUMMY_SP, + })); + witness + }).collect() + } else { + pats.into_iter().flat_map(|witness| { + missing_ctors.iter().map(move |ctor| { + witness.clone().push_wild_constructor(cx, ctor, pcx.ty) + }) + }).collect() + }; + UsefulWithWitness(new_witnesses) } result => result } diff --git a/src/test/compile-fail/match-slice-patterns.rs b/src/test/compile-fail/match-slice-patterns.rs index c0fc75f9713a..fd4bd1c7b944 100644 --- a/src/test/compile-fail/match-slice-patterns.rs +++ b/src/test/compile-fail/match-slice-patterns.rs @@ -12,7 +12,7 @@ fn check(list: &[Option<()>]) { match list { - //~^ ERROR `&[None, Some(_), None, _]` and `&[Some(_), Some(_), None, _]` not covered + //~^ ERROR `&[_, Some(_), None, _]` not covered &[] => {}, &[_] => {}, &[_, _] => {}, diff --git a/src/test/compile-fail/uninhabited-irrefutable.rs b/src/test/compile-fail/uninhabited-irrefutable.rs new file mode 100644 index 000000000000..4755fdd4fd5e --- /dev/null +++ b/src/test/compile-fail/uninhabited-irrefutable.rs @@ -0,0 +1,38 @@ +// Copyright 2012 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(never_type)] + +mod foo { + pub struct SecretlyEmpty { + _priv: !, + } + + pub struct NotSoSecretlyEmpty { + pub _pub: !, + } +} + +struct NotSoSecretlyEmpty { + _priv: !, +} + +enum Foo { + A(foo::SecretlyEmpty), + B(foo::NotSoSecretlyEmpty), + C(NotSoSecretlyEmpty), + D(u32), +} + +fn main() { + let x: Foo = Foo::D(123); + let Foo::D(_y) = x; //~ ERROR refutable pattern in local binding: `A(_)` not covered +} + diff --git a/src/test/compile-fail/uninhabited-patterns.rs b/src/test/compile-fail/uninhabited-patterns.rs new file mode 100644 index 000000000000..0de29f3a8d73 --- /dev/null +++ b/src/test/compile-fail/uninhabited-patterns.rs @@ -0,0 +1,49 @@ +// Copyright 2012 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(box_patterns)] +#![feature(slice_patterns)] +#![feature(box_syntax)] +#![feature(never_type)] +#![deny(unreachable_patterns)] + +mod foo { + pub struct SecretlyEmpty { + _priv: !, + } +} + +struct NotSoSecretlyEmpty { + _priv: !, +} + +fn main() { + let x: &[!] = &[]; + + match x { + &[] => (), + &[..] => (), //~ ERROR unreachable pattern + }; + + let x: Result, &[Result]> = Err(&[]); + match x { + Ok(box _) => (), //~ ERROR unreachable pattern + Err(&[]) => (), + Err(&[..]) => (), //~ ERROR unreachable pattern + } + + let x: Result> = Err(Err(123)); + match x { + Ok(_y) => (), + Err(Err(_y)) => (), + Err(Ok(_y)) => (), //~ ERROR unreachable pattern + } +} + diff --git a/src/test/ui/check_match/issue-35609.stderr b/src/test/ui/check_match/issue-35609.stderr index 66069c7a86a3..0aafe3f17b3d 100644 --- a/src/test/ui/check_match/issue-35609.stderr +++ b/src/test/ui/check_match/issue-35609.stderr @@ -4,11 +4,11 @@ error[E0004]: non-exhaustive patterns: `(B, _)`, `(C, _)`, `(D, _)` and 2 more n 20 | match (A, ()) { | ^^^^^^^ patterns `(B, _)`, `(C, _)`, `(D, _)` and 2 more not covered -error[E0004]: non-exhaustive patterns: `(A, B)`, `(B, B)`, `(C, B)` and 27 more not covered +error[E0004]: non-exhaustive patterns: `(_, B)`, `(_, C)`, `(_, D)` and 2 more not covered --> $DIR/issue-35609.rs:24:11 | 24 | match (A, A) { - | ^^^^^^ patterns `(A, B)`, `(B, B)`, `(C, B)` and 27 more not covered + | ^^^^^^ patterns `(_, B)`, `(_, C)`, `(_, D)` and 2 more not covered error[E0004]: non-exhaustive patterns: `((B, _), _)`, `((C, _), _)`, `((D, _), _)` and 2 more not covered --> $DIR/issue-35609.rs:28:11