Make is_useful handle empty types properly

Author: canndrew

src/librustc/lint/builtin.rs

@@ -70,6 +70,12 @@ declare_lint! {
     "detects unreachable code paths"
 }
 
+declare_lint! {
+    pub UNREACHABLE_PATTERNS,
+    Warn,
+    "detects unreachable patterns"
+}
+
 declare_lint! {
     pub WARNINGS,
     Warn,
@@ -239,6 +245,7 @@ impl LintPass for HardwiredLints {
             UNUSED_ASSIGNMENTS,
             DEAD_CODE,
             UNREACHABLE_CODE,
+            UNREACHABLE_PATTERNS,
             WARNINGS,
             UNUSED_FEATURES,
             STABLE_FEATURES,

src/librustc_const_eval/_match.rs

@@ -17,7 +17,7 @@ use eval::{compare_const_vals};
 
 use rustc_const_math::ConstInt;
 
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::indexed_vec::Idx;
 
 use pattern::{FieldPattern, Pattern, PatternKind};
@@ -29,6 +29,7 @@ use rustc::ty::{self, Ty, TyCtxt, TypeFoldable};
 use rustc::mir::Field;
 use rustc::util::common::ErrorReported;
 
+use syntax::ast::NodeId;
 use syntax_pos::{Span, DUMMY_SP};
 
 use arena::TypedArena;
@@ -144,6 +145,14 @@ impl<'a, 'tcx> FromIterator<Vec<&'a Pattern<'tcx>>> for Matrix<'a, 'tcx> {
 //NOTE: appears to be the only place other then InferCtxt to contain a ParamEnv
 pub struct MatchCheckCtxt<'a, 'tcx: 'a> {
     pub tcx: TyCtxt<'a, 'tcx, 'tcx>,
+    /// (roughly) where in the code the match occurs. This is necessary for
+    /// checking inhabited-ness of types because whether a type is (visibly)
+    /// inhabited can depend on whether it was defined in the current module or
+    /// not. eg.
+    ///     struct Foo { _private: ! }
+    /// can not be seen to be empty outside it's module and should not
+    /// be matchable with an empty match statement.
+    pub node: NodeId,
     /// A wild pattern with an error type - it exists to avoid having to normalize
     /// associated types to get field types.
     pub wild_pattern: &'a Pattern<'tcx>,
@@ -154,6 +163,7 @@ pub struct MatchCheckCtxt<'a, 'tcx: 'a> {
 impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> {
     pub fn create_and_enter<F, R>(
         tcx: TyCtxt<'a, 'tcx, 'tcx>,
+        node: NodeId,
         f: F) -> R
         where F: for<'b> FnOnce(MatchCheckCtxt<'b, 'tcx>) -> R
     {
@@ -167,6 +177,7 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> {
 
         f(MatchCheckCtxt {
             tcx: tcx,
+            node: node,
             wild_pattern: &wild_pattern,
             pattern_arena: &pattern_arena,
             byte_array_map: FxHashMap(),
@@ -362,9 +373,9 @@ impl<'tcx> Witness<'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 the return value.
-fn missing_constructors(cx: &mut MatchCheckCtxt,
-                        matrix: &Matrix,
-                        pcx: PatternContext) -> Vec<Constructor> {
+fn missing_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
+                                      matrix: &Matrix,
+                                      pcx: PatternContext<'tcx>) -> Vec<Constructor> {
     let used_constructors: Vec<Constructor> =
         matrix.0.iter()
         .flat_map(|row| pat_constructors(cx, row[0], pcx).unwrap_or(vec![]))
@@ -384,16 +395,46 @@ fn missing_constructors(cx: &mut MatchCheckCtxt,
 ///
 /// but is instead bounded by the maximum fixed length of slice patterns in
 /// the column of patterns being analyzed.
-fn all_constructors(_cx: &mut MatchCheckCtxt, pcx: PatternContext) -> Vec<Constructor> {
+///
+/// We make sure to omit constructors that are statically impossible. eg for
+/// Option<!> we do not include Some(_) in the returned list of constructors.
+fn all_constructors<'a, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
+                                  pcx: PatternContext<'tcx>) -> Vec<Constructor>
+{
     match pcx.ty.sty {
         ty::TyBool =>
             [true, false].iter().map(|b| ConstantValue(ConstVal::Bool(*b))).collect(),
-        ty::TySlice(_) =>
-            (0..pcx.max_slice_length+1).map(|length| Slice(length)).collect(),
-        ty::TyArray(_, length) => vec![Slice(length)],
-        ty::TyAdt(def, _) if def.is_enum() && def.variants.len() > 1 =>
-            def.variants.iter().map(|v| Variant(v.did)).collect(),
-        _ => vec![Single]
+        ty::TySlice(ref sub_ty) => {
+            if sub_ty.is_uninhabited(Some(cx.node), cx.tcx) {
+                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(Some(cx.node), cx.tcx) {
+                vec![Slice(length)]
+            } else {
+                vec![]
+            }
+        }
+        ty::TyAdt(def, substs) if def.is_enum() && def.variants.len() != 1 => {
+            def.variants.iter().filter_map(|v| {
+                let mut visited = FxHashSet::default();
+                if v.is_uninhabited_recurse(&mut visited, Some(cx.node), cx.tcx, substs, false) {
+                    None
+                } else {
+                    Some(Variant(v.did))
+                }
+            }).collect()
+        }
+        _ => {
+            if pcx.ty.is_uninhabited(Some(cx.node), cx.tcx) {
+                vec![]
+            } else {
+                vec![Single]
+            }
+        }
     }
 }
 

src/librustc_const_eval/check_match.rs

@@ -25,6 +25,7 @@ use rustc::middle::mem_categorization::{cmt};
 use rustc::session::Session;
 use rustc::traits::Reveal;
 use rustc::ty::{self, TyCtxt};
+use rustc::lint;
 use rustc_errors::DiagnosticBuilder;
 
 use rustc::hir::def::*;
@@ -150,7 +151,7 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
             }
         }
 
-        MatchCheckCtxt::create_and_enter(self.tcx, |ref mut cx| {
+        MatchCheckCtxt::create_and_enter(self.tcx, scrut.id, |ref mut cx| {
             let mut have_errors = false;
 
             let inlined_arms : Vec<(Vec<_>, _)> = arms.iter().map(|arm| (
@@ -210,7 +211,7 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
             "local binding"
         };
 
-        MatchCheckCtxt::create_and_enter(self.tcx, |ref mut cx| {
+        MatchCheckCtxt::create_and_enter(self.tcx, pat.id, |ref mut cx| {
             let mut patcx = PatternContext::new(self.tcx);
             let pats : Matrix = vec![vec![
                 expand_pattern(cx, patcx.lower_pattern(pat))
@@ -324,14 +325,19 @@ fn check_arms<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
                         },
 
                         hir::MatchSource::Normal => {
-                            let mut err = struct_span_err!(cx.tcx.sess, pat.span, E0001,
-                                                           "unreachable pattern");
-                            err.span_label(pat.span, &"this is an unreachable pattern");
-                            // if we had a catchall pattern, hint at that
+                            // if we had a catchall pattern, raise an error.
+                            // Otherwise an unreachable pattern raises a warning.
                             if let Some(catchall) = catchall {
+                                let mut err = struct_span_err!(cx.tcx.sess, pat.span, E0001,
+                                                               "unreachable pattern");
+                                err.span_label(pat.span, &"this is an unreachable pattern");
                                 err.span_note(catchall, "this pattern matches any value");
+                                err.emit();
+                            } else {
+                                cx.tcx.sess.add_lint(lint::builtin::UNREACHABLE_PATTERNS,
+                                                     hir_pat.id, pat.span,
+                                                     String::from("unreachable pattern"));
                             }
-                            err.emit();
                         },
 
                         hir::MatchSource::TryDesugar => {

src/librustc_const_eval/diagnostics.rs

@@ -28,7 +28,7 @@ For example, the following `match` block has too many arms:
 ```compile_fail,E0001
 match Some(0) {
     Some(bar) => {/* ... */}
-    None => {/* ... */}
+    x => {/* ... */} // This handles the `None` case
     _ => {/* ... */} // All possible cases have already been handled
 }
 ```

src/librustc_lint/lib.rs

@@ -165,6 +165,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
                     DEAD_CODE,
                     UNUSED_MUT,
                     UNREACHABLE_CODE,
+                    UNREACHABLE_PATTERNS,
                     UNUSED_MUST_USE,
                     UNUSED_UNSAFE,
                     PATH_STATEMENTS,

src/librustc_typeck/check/_match.rs

@@ -399,7 +399,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 self.check_pat(&p, discrim_ty);
                 all_pats_diverge &= self.diverges.get();
             }
-            all_pats_diverge
+            // As discussed with @eddyb, this is for disabling unreachable_code
+            // warnings on patterns (they're now subsumed by unreachable_patterns
+            // warnings).
+            match all_pats_diverge {
+                Diverges::Maybe => Diverges::Maybe,
+                Diverges::Always | Diverges::WarnedAlways => Diverges::WarnedAlways,
+            }
         }).collect();
 
         // Now typecheck the blocks.

src/test/compile-fail/E0001.rs

@@ -8,11 +8,13 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+#![deny(unreachable_patterns)]
+
 fn main() {
     let foo = Some(1);
     match foo {
-        Some(bar) => {/* ... */}
+        Some(_) => {/* ... */}
         None => {/* ... */}
-        _ => {/* ... */} //~ ERROR E0001
+        _ => {/* ... */} //~ ERROR unreachable pattern
     }
 }

src/test/compile-fail/issue-12116.rs

@@ -20,7 +20,7 @@ fn tail(source_list: &IntList) -> IntList {
     match source_list {
         &IntList::Cons(val, box ref next_list) => tail(next_list),
         &IntList::Cons(val, box Nil)           => IntList::Cons(val, box Nil),
-//~^ ERROR unreachable pattern
+//~^ ERROR cannot move out of borrowed content
 //~^^ WARN pattern binding `Nil` is named the same as one of the variants of the type `IntList`
         _                          => panic!()
     }

src/test/compile-fail/issue-12369.rs

@@ -9,6 +9,8 @@
 // except according to those terms.
 
 #![feature(slice_patterns)]
+#![allow(unused_variables)]
+#![deny(unreachable_patterns)]
 
 fn main() {
     let sl = vec![1,2,3];

src/test/compile-fail/issue-13727.rs

@@ -8,6 +8,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+#![allow(overflowing_literals)]
+#![deny(unreachable_patterns)]
+
 fn test(val: u8) {
   match val {
     256 => print!("0b1110\n"),