Rollup merge of #39980 - arielb1:privately-uninhabited, r=nikomatsakis

check_match: don't treat privately uninhabited types as uninhabited

Fixes #38972, which is a regression in 1.16 from @canndrew's patchset.

r? @nikomatsakis

beta-nominating because regression.

5 files changed, 154 insertions, 37 deletions

diff --git a/src/librustc_const_eval/_match.rs b/src/librustc_const_eval/_match.rs
index 78c4027aa43..5a9f885719c 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<Constructor>
 {
-    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<Constructor> = 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 6f33b4fad76..9b30946c0be 100644
--- a/src/librustc_const_eval/check_match.rs
+++ b/src/librustc_const_eval/check_match.rs
@@ -177,6 +177,31 @@ 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() {
+                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 {} \
+                                                        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())
@@ -188,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/match-privately-empty.rs b/src/test/compile-fail/match-privately-empty.rs
new file mode 100644
index 00000000000..3affb1c03e9
--- /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 <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(never_type)]
+
+mod private {
+    pub struct Private {
+        _bot: !,
+        pub misc: bool,
+    }
+    pub const DATA: Option<Private> = 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 0f8b0a6c238..0c3ea53a903 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 {}; //~ ERROR non-exhaustive
 
     let x: (Void,) = unsafe { std::mem::uninitialized() };
-    let _ = match x {};
-    //~^ ERROR non-exhaustive
+    let _ = match x {}; //~ ERROR non-exhaustive
 
     let x: [Void; 1] = unsafe { std::mem::uninitialized() };
-    let _ = match x {};
-    //~^ ERROR non-exhaustive
+    let _ = match x {}; //~ ERROR non-exhaustive
 
     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 00000000000..d5df84e0fb0
--- /dev/null
+++ b/src/test/run-pass/issue-38972.rs
@@ -0,0 +1,25 @@
+// 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 <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.
+
+// 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<Foo> { None }
+
+#[deny(warnings)]
+fn main() {
+    match make_foo() {
+        None => {},
+        Some(_) => {}
+    }
+}