From a84eb95a7db9460d317eeef62da106459a346f0b Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 20 Feb 2017 19:18:31 +0200 Subject: [PATCH 1/2] check_match: don't treat privately uninhabited types as uninhabited Fixes #38972. --- src/librustc_const_eval/_match.rs | 92 +++++++++++++------ src/librustc_const_eval/check_match.rs | 20 ++++ .../compile-fail/match-privately-empty.rs | 30 ++++++ .../uninhabited-matches-feature-gated.rs | 10 +- src/test/run-pass/issue-38972.rs | 29 ++++++ 5 files changed, 144 insertions(+), 37 deletions(-) create mode 100644 src/test/compile-fail/match-privately-empty.rs create mode 100644 src/test/run-pass/issue-38972.rs diff --git a/src/librustc_const_eval/_match.rs b/src/librustc_const_eval/_match.rs index 78c4027aa4319..5a9f885719c8f 100644 --- a/src/librustc_const_eval/_match.rs +++ b/src/librustc_const_eval/_match.rs @@ -196,6 +196,28 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { } }).clone() } + + fn is_uninhabited(&self, ty: Ty<'tcx>) -> bool { + if self.tcx.sess.features.borrow().never_type { + ty.is_uninhabited_from(self.module, self.tcx) + } else { + false + } + } + + fn is_variant_uninhabited(&self, + variant: &'tcx ty::VariantDef, + substs: &'tcx ty::subst::Substs<'tcx>) -> bool + { + if self.tcx.sess.features.borrow().never_type { + let forest = variant.uninhabited_from( + &mut FxHashMap::default(), self.tcx, substs, AdtKind::Enum + ); + forest.contains(self.tcx, self.module) + } else { + false + } + } } #[derive(Clone, Debug, PartialEq)] @@ -379,48 +401,32 @@ impl<'tcx> Witness<'tcx> { fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, pcx: PatternContext<'tcx>) -> Vec { - let check_inhabited = cx.tcx.sess.features.borrow().never_type; debug!("all_constructors({:?})", pcx.ty); match pcx.ty.sty { ty::TyBool => [true, false].iter().map(|b| ConstantValue(ConstVal::Bool(*b))).collect(), ty::TySlice(ref sub_ty) => { - if sub_ty.is_uninhabited_from(cx.module, cx.tcx) - && check_inhabited - { + if cx.is_uninhabited(sub_ty) { vec![Slice(0)] } else { (0..pcx.max_slice_length+1).map(|length| Slice(length)).collect() } } ty::TyArray(ref sub_ty, length) => { - if length == 0 || !(sub_ty.is_uninhabited_from(cx.module, cx.tcx) - && check_inhabited) - { - vec![Slice(length)] - } else { + if length > 0 && cx.is_uninhabited(sub_ty) { vec![] + } else { + vec![Slice(length)] } } ty::TyAdt(def, substs) if def.is_enum() && def.variants.len() != 1 => { - def.variants.iter().filter_map(|v| { - let mut visited = FxHashMap::default(); - let forest = v.uninhabited_from(&mut visited, - cx.tcx, substs, - AdtKind::Enum); - if forest.contains(cx.tcx, cx.module) - && check_inhabited - { - None - } else { - Some(Variant(v.did)) - } - }).collect() + def.variants.iter() + .filter(|v| !cx.is_variant_uninhabited(v, substs)) + .map(|v| Variant(v.did)) + .collect() } _ => { - if pcx.ty.is_uninhabited_from(cx.module, cx.tcx) - && check_inhabited - { + if cx.is_uninhabited(pcx.ty) { vec![] } else { vec![Single] @@ -564,7 +570,6 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, 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), @@ -590,7 +595,6 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, 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 @@ -599,8 +603,23 @@ pub fn is_useful<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>, // 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() { + // + // However, if our scrutinee is *privately* an empty enum, we + // must treat it as though it had an "unknown" constructor (in + // that case, all other patterns obviously can't be variants) + // to avoid exposing its emptyness. See the `match_privately_empty` + // test for details. + // + // FIXME: currently the only way I know of something can + // be a privately-empty enum is when the never_type + // feature flag is not present, so this is only + // needed for that case. + + let is_privately_empty = + all_ctors.is_empty() && !cx.is_uninhabited(pcx.ty); + debug!("missing_ctors={:?} is_privately_empty={:?}", missing_ctors, + is_privately_empty); + if missing_ctors.is_empty() && !is_privately_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) @@ -649,6 +668,7 @@ fn is_useful_specialized<'p, 'a:'p, 'tcx: 'a>( lty: Ty<'tcx>, witness: WitnessPreference) -> Usefulness<'tcx> { + debug!("is_useful_specialized({:?}, {:?}, {:?})", v, ctor, lty); let sub_pat_tys = constructor_sub_pattern_tys(cx, &ctor, lty); let wild_patterns_owned: Vec<_> = sub_pat_tys.iter().map(|ty| { Pattern { @@ -754,7 +774,19 @@ fn constructor_sub_pattern_tys<'a, 'tcx: 'a>(cx: &MatchCheckCtxt<'a, 'tcx>, ty::TyRef(_, ref ty_and_mut) => vec![ty_and_mut.ty], ty::TyAdt(adt, substs) => { adt.variants[ctor.variant_index_for_adt(adt)].fields.iter().map(|field| { - field.ty(cx.tcx, substs) + let is_visible = adt.is_enum() + || field.vis.is_accessible_from(cx.module, cx.tcx); + if is_visible { + field.ty(cx.tcx, substs) + } else { + // Treat all non-visible fields as nil. They + // can't appear in any other pattern from + // this match (because they are private), + // so their type does not matter - but + // we don't want to know they are + // uninhabited. + cx.tcx.mk_nil() + } }).collect() } _ => vec![], diff --git a/src/librustc_const_eval/check_match.rs b/src/librustc_const_eval/check_match.rs index 6f33b4fad769f..e5362a70e53b6 100644 --- a/src/librustc_const_eval/check_match.rs +++ b/src/librustc_const_eval/check_match.rs @@ -177,6 +177,26 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> { // Fourth, check for unreachable arms. check_arms(cx, &inlined_arms, source); + // Then, if the match has no arms, check whether the scrutinee + // is uninhabited. + let pat_ty = self.tables.node_id_to_type(scrut.id); + let module = self.tcx.hir.local_def_id(self.tcx.hir.get_module_parent(scrut.id)); + if inlined_arms.is_empty() { + if !pat_ty.is_uninhabited_from(module, self.tcx) { + // We know the type is inhabited, so this must be wrong + let mut err = create_e0004(self.tcx.sess, scrut.span, + format!("non-exhaustive patterns: type {} \ + is non-empty", + pat_ty)); + span_help!(&mut err, scrut.span, + "Please ensure that all possible cases are being handled; \ + possibly adding wildcards or more match arms."); + err.emit(); + } + // If the type *is* uninhabited, it's vacuously exhaustive + return; + } + let matrix: Matrix = inlined_arms .iter() .filter(|&&(_, guard)| guard.is_none()) diff --git a/src/test/compile-fail/match-privately-empty.rs b/src/test/compile-fail/match-privately-empty.rs new file mode 100644 index 0000000000000..3affb1c03e952 --- /dev/null +++ b/src/test/compile-fail/match-privately-empty.rs @@ -0,0 +1,30 @@ +// Copyright 2017 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 private { + pub struct Private { + _bot: !, + pub misc: bool, + } + pub const DATA: Option = None; +} + +fn main() { + match private::DATA { + //~^ ERROR non-exhaustive patterns: `Some(Private { misc: true, .. })` not covered + None => {} + Some(private::Private { + misc: false, + .. + }) => {} + } +} diff --git a/src/test/compile-fail/uninhabited-matches-feature-gated.rs b/src/test/compile-fail/uninhabited-matches-feature-gated.rs index 0f8b0a6c238d0..68781a407cc00 100644 --- a/src/test/compile-fail/uninhabited-matches-feature-gated.rs +++ b/src/test/compile-fail/uninhabited-matches-feature-gated.rs @@ -19,16 +19,13 @@ fn main() { }; let x: &Void = unsafe { std::mem::uninitialized() }; - let _ = match x {}; - //~^ ERROR non-exhaustive + let _ = match x {}; // okay let x: (Void,) = unsafe { std::mem::uninitialized() }; - let _ = match x {}; - //~^ ERROR non-exhaustive + let _ = match x {}; // okay let x: [Void; 1] = unsafe { std::mem::uninitialized() }; - let _ = match x {}; - //~^ ERROR non-exhaustive + let _ = match x {}; // okay let x: &[Void] = unsafe { std::mem::uninitialized() }; let _ = match x { //~ ERROR non-exhaustive @@ -47,4 +44,3 @@ fn main() { let Ok(x) = x; //~^ ERROR refutable } - diff --git a/src/test/run-pass/issue-38972.rs b/src/test/run-pass/issue-38972.rs new file mode 100644 index 0000000000000..9c440503214e6 --- /dev/null +++ b/src/test/run-pass/issue-38972.rs @@ -0,0 +1,29 @@ +// Copyright 2017 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. + +// This issue tracks a regression (a new warning) without +// feature(never_type). When we make that the default, please +// remove this test. + +enum Foo { } + +fn make_foo() -> Option { None } + +fn use_foo(v: &Foo) -> ! { + match v { } +} + +#[deny(warnings)] +fn main() { + match make_foo() { + None => {}, + Some(ref v) => use_foo(v), + } +} From 87e544bca5ae1bd187f11239abcab1f73c836049 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 23 Feb 2017 22:46:19 +0200 Subject: [PATCH 2/2] use a more conservative inhabitableness rule This is a [breaking-change] from 1.15, because this used to compile: ```Rust enum Void {} fn foo(x: &Void) { match x {} } ``` --- src/librustc_const_eval/check_match.rs | 16 +++++++++++++++- .../uninhabited-matches-feature-gated.rs | 6 +++--- src/test/run-pass/issue-38972.rs | 6 +----- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/librustc_const_eval/check_match.rs b/src/librustc_const_eval/check_match.rs index e5362a70e53b6..9b30946c0bebb 100644 --- a/src/librustc_const_eval/check_match.rs +++ b/src/librustc_const_eval/check_match.rs @@ -182,7 +182,12 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> { let pat_ty = self.tables.node_id_to_type(scrut.id); let module = self.tcx.hir.local_def_id(self.tcx.hir.get_module_parent(scrut.id)); if inlined_arms.is_empty() { - if !pat_ty.is_uninhabited_from(module, self.tcx) { + let scrutinee_is_uninhabited = if self.tcx.sess.features.borrow().never_type { + pat_ty.is_uninhabited_from(module, self.tcx) + } else { + self.conservative_is_uninhabited(pat_ty) + }; + if !scrutinee_is_uninhabited { // We know the type is inhabited, so this must be wrong let mut err = create_e0004(self.tcx.sess, scrut.span, format!("non-exhaustive patterns: type {} \ @@ -208,6 +213,15 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> { }) } + fn conservative_is_uninhabited(&self, scrutinee_ty: Ty<'tcx>) -> bool { + // "rustc-1.0-style" uncontentious uninhabitableness check + match scrutinee_ty.sty { + ty::TyNever => true, + ty::TyAdt(def, _) => def.variants.is_empty(), + _ => false + } + } + fn check_irrefutable(&self, pat: &Pat, is_fn_arg: bool) { let origin = if is_fn_arg { "function argument" diff --git a/src/test/compile-fail/uninhabited-matches-feature-gated.rs b/src/test/compile-fail/uninhabited-matches-feature-gated.rs index 68781a407cc00..0c3ea53a903ae 100644 --- a/src/test/compile-fail/uninhabited-matches-feature-gated.rs +++ b/src/test/compile-fail/uninhabited-matches-feature-gated.rs @@ -19,13 +19,13 @@ fn main() { }; let x: &Void = unsafe { std::mem::uninitialized() }; - let _ = match x {}; // okay + let _ = match x {}; //~ ERROR non-exhaustive let x: (Void,) = unsafe { std::mem::uninitialized() }; - let _ = match x {}; // okay + let _ = match x {}; //~ ERROR non-exhaustive let x: [Void; 1] = unsafe { std::mem::uninitialized() }; - let _ = match x {}; // okay + let _ = match x {}; //~ ERROR non-exhaustive let x: &[Void] = unsafe { std::mem::uninitialized() }; let _ = match x { //~ ERROR non-exhaustive diff --git a/src/test/run-pass/issue-38972.rs b/src/test/run-pass/issue-38972.rs index 9c440503214e6..d5df84e0fb083 100644 --- a/src/test/run-pass/issue-38972.rs +++ b/src/test/run-pass/issue-38972.rs @@ -16,14 +16,10 @@ enum Foo { } fn make_foo() -> Option { None } -fn use_foo(v: &Foo) -> ! { - match v { } -} - #[deny(warnings)] fn main() { match make_foo() { None => {}, - Some(ref v) => use_foo(v), + Some(_) => {} } }