Rollup merge of #65414 - davidtwco:issue-65157-non-exhaustive-always-useful, r=varkor

ignore uninhabited non-exhaustive variant fields

Fixes #65157.

This PR modifies the uninhabitedness checking so that the fields of
a non-exhaustive variant (which is not local) are ignored if they are
uninhabited. This is an improvement over the previous behaviour which
considered all non-local non-exhaustive variants useful because
unreachable patterns are now detected.

r? @arielb1
cc @varkor

src/librustc_mir/hair/pattern/_match.rs

@@ -394,16 +394,6 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> {
         }
     }
 
-    fn is_non_exhaustive_variant<'p>(&self, pattern: &'p Pat<'tcx>) -> bool {
-        match *pattern.kind {
-            PatKind::Variant { adt_def, variant_index, .. } => {
-                let ref variant = adt_def.variants[variant_index];
-                variant.is_field_list_non_exhaustive()
-            }
-            _ => false,
-        }
-    }
-
     fn is_non_exhaustive_enum(&self, ty: Ty<'tcx>) -> bool {
         match ty.kind {
             ty::Adt(adt_def, ..) => adt_def.is_variant_list_non_exhaustive(),
@@ -1252,19 +1242,12 @@ pub fn is_useful<'p, 'a, 'tcx>(
     debug!("is_useful_expand_first_col: pcx={:#?}, expanding {:#?}", pcx, v[0]);
 
     if let Some(constructors) = pat_constructors(cx, v[0], pcx) {
-        let is_declared_nonexhaustive = cx.is_non_exhaustive_variant(v[0]) && !cx.is_local(pcx.ty);
+        debug!("is_useful - expanding constructors: {:#?}", constructors);
-        debug!("is_useful - expanding constructors: {:#?}, is_declared_nonexhaustive: {:?}",
+        split_grouped_constructors(
-               constructors, is_declared_nonexhaustive);
+            cx.tcx, cx.param_env, constructors, matrix, pcx.ty, pcx.span, Some(hir_id),
-
+        ).into_iter().map(|c|
-        if is_declared_nonexhaustive {
+            is_useful_specialized(cx, matrix, v, c, pcx.ty, witness, hir_id)
-            Useful
+        ).find(|result| result.is_useful()).unwrap_or(NotUseful)
-        } else {
-            split_grouped_constructors(
-                cx.tcx, cx.param_env, constructors, matrix, pcx.ty, pcx.span, Some(hir_id),
-            ).into_iter().map(|c|
-                is_useful_specialized(cx, matrix, v, c, pcx.ty, witness, hir_id)
-            ).find(|result| result.is_useful()).unwrap_or(NotUseful)
-        }
     } else {
         debug!("is_useful - expanding wildcard");
 
@@ -1548,27 +1531,30 @@ fn constructor_sub_pattern_tys<'a, 'tcx>(
                 // Use T as the sub pattern type of Box<T>.
                 vec![substs.type_at(0)]
             } else {
-                adt.variants[ctor.variant_index_for_adt(cx, adt)].fields.iter().map(|field| {
+                let variant = &adt.variants[ctor.variant_index_for_adt(cx, adt)];
+                let is_non_exhaustive = variant.is_field_list_non_exhaustive() && !cx.is_local(ty);
+                variant.fields.iter().map(|field| {
                     let is_visible = adt.is_enum()
                         || field.vis.is_accessible_from(cx.module, cx.tcx);
-                    if is_visible {
+                    let is_uninhabited = cx.is_uninhabited(field.ty(cx.tcx, substs));
-                        let ty = field.ty(cx.tcx, substs);
+                    match (is_visible, is_non_exhaustive, is_uninhabited) {
-                        match ty.kind {
+                        // Treat all uninhabited types in non-exhaustive variants as `TyErr`.
-                            // If the field type returned is an array of an unknown
+                        (_, true, true) => cx.tcx.types.err,
-                            // size return an TyErr.
+                        // Treat all non-visible fields as `TyErr`. They can't appear in any
-                            ty::Array(_, len)
+                        // other pattern from this match (because they are private), so their
-                                if len.try_eval_usize(cx.tcx, cx.param_env).is_none() =>
+                        // type does not matter - but we don't want to know they are uninhabited.
-                                cx.tcx.types.err,
+                        (false, ..) => cx.tcx.types.err,
-                            _ => ty,
+                        (true, ..) => {
-                        }
+                            let ty = field.ty(cx.tcx, substs);
-                    } else {
+                            match ty.kind {
-                        // Treat all non-visible fields as TyErr. They
+                                // If the field type returned is an array of an unknown
-                        // can't appear in any other pattern from
+                                // size return an TyErr.
-                        // this match (because they are private),
+                                ty::Array(_, len)
-                        // so their type does not matter - but
+                                    if len.try_eval_usize(cx.tcx, cx.param_env).is_none() =>
-                        // we don't want to know they are
+                                    cx.tcx.types.err,
-                        // uninhabited.
+                                _ => ty,
-                        cx.tcx.types.err
+                            }
+                        },
                     }
                 }).collect()
             }
@@ -1874,15 +1860,18 @@ fn constructor_covered_by_range<'tcx>(
     }
 }
 
-fn patterns_for_variant<'p, 'tcx>(
+fn patterns_for_variant<'p, 'a: 'p, 'tcx>(
+    cx: &mut MatchCheckCtxt<'a, 'tcx>,
     subpatterns: &'p [FieldPat<'tcx>],
-    wild_patterns: &[&'p Pat<'tcx>])
+    wild_patterns: &[&'p Pat<'tcx>],
-    -> SmallVec<[&'p Pat<'tcx>; 2]>
+    is_non_exhaustive: bool,
-{
+) -> SmallVec<[&'p Pat<'tcx>; 2]> {
     let mut result = SmallVec::from_slice(wild_patterns);
 
     for subpat in subpatterns {
-        result[subpat.field.index()] = &subpat.pattern;
+        if !is_non_exhaustive || !cx.is_uninhabited(subpat.pattern.ty) {
+            result[subpat.field.index()] = &subpat.pattern;
+        }
     }
 
     debug!("patterns_for_variant({:#?}, {:#?}) = {:#?}", subpatterns, wild_patterns, result);
@@ -1916,13 +1905,14 @@ fn specialize<'p, 'a: 'p, 'tcx>(
 
         PatKind::Variant { adt_def, variant_index, ref subpatterns, .. } => {
             let ref variant = adt_def.variants[variant_index];
+            let is_non_exhaustive = variant.is_field_list_non_exhaustive() && !cx.is_local(pat.ty);
             Some(Variant(variant.def_id))
                 .filter(|variant_constructor| variant_constructor == constructor)
-                .map(|_| patterns_for_variant(subpatterns, wild_patterns))
+                .map(|_| patterns_for_variant(cx, subpatterns, wild_patterns, is_non_exhaustive))
         }
 
         PatKind::Leaf { ref subpatterns } => {
-            Some(patterns_for_variant(subpatterns, wild_patterns))
+            Some(patterns_for_variant(cx, subpatterns, wild_patterns, false))
         }
 
         PatKind::Deref { ref subpattern } => {

src/test/ui/rfc-2008-non-exhaustive/uninhabited/issue-65157-repeated-match-arm.rs

@@ -0,0 +1,22 @@
+// aux-build:uninhabited.rs
+#![deny(unreachable_patterns)]
+#![feature(never_type)]
+#![feature(non_exhaustive)]
+
+extern crate uninhabited;
+
+use uninhabited::PartiallyInhabitedVariants;
+
+// This test checks a redundant/useless pattern of a non-exhaustive enum/variant is still
+// warned against.
+
+pub fn foo(x: PartiallyInhabitedVariants) {
+    match x {
+        PartiallyInhabitedVariants::Struct { .. } => {},
+        PartiallyInhabitedVariants::Struct { .. } => {},
+        //~^ ERROR unreachable pattern
+        _ => {},
+    }
+}
+
+fn main() { }

src/test/ui/rfc-2008-non-exhaustive/uninhabited/issue-65157-repeated-match-arm.stderr

@@ -0,0 +1,14 @@
+error: unreachable pattern
+  --> $DIR/issue-65157-repeated-match-arm.rs:16:9
+   |
+LL |         PartiallyInhabitedVariants::Struct { .. } => {},
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/issue-65157-repeated-match-arm.rs:2:9
+   |
+LL | #![deny(unreachable_patterns)]
+   |         ^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+