Auto merge of rust-lang#89105 - DevinR528:reachable-fix, r=Nadrieril

Fix: non_exhaustive_omitted_patterns by filtering unstable and doc hidden variants

Fixes: rust-lang#89042

Now that rust-lang#86809 has been merged there are cases (std::io::ErrorKind) where unstable feature gated variants were included in warning/error messages when the feature was not turned on. This filters those variants out of the return of `SplitWildcard::new`.

Variants marked `doc(hidden)` are filtered out of the witnesses list in `Usefulness::apply_constructor`.

Probably worth a perf run :shrug: since this area can be sensitive.

Author: bors

compiler/rustc_middle/src/ty/mod.rs

@@ -38,7 +38,7 @@ use rustc_macros::HashStable;
 use rustc_query_system::ich::StableHashingContext;
 use rustc_session::cstore::CrateStoreDyn;
 use rustc_span::symbol::{kw, Ident, Symbol};
-use rustc_span::Span;
+use rustc_span::{sym, Span};
 use rustc_target::abi::Align;
 
 use std::cmp::Ordering;
@@ -1900,6 +1900,14 @@ impl<'tcx> TyCtxt<'tcx> {
         self.sess.contains_name(&self.get_attrs(did), attr)
     }
 
+    /// Determines whether an item is annotated with `doc(hidden)`.
+    pub fn is_doc_hidden(self, did: DefId) -> bool {
+        self.get_attrs(did)
+            .iter()
+            .filter_map(|attr| if attr.has_name(sym::doc) { attr.meta_item_list() } else { None })
+            .any(|items| items.iter().any(|item| item.has_name(sym::hidden)))
+    }
+
     /// Returns `true` if this is an `auto trait`.
     pub fn trait_is_auto(self, trait_def_id: DefId) -> bool {
         self.trait_def(trait_def_id).has_auto_impl

compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs

@@ -52,11 +52,11 @@ use rustc_data_structures::captures::Captures;
 use rustc_index::vec::Idx;
 
 use rustc_hir::{HirId, RangeEnd};
-use rustc_middle::mir::interpret::ConstValue;
 use rustc_middle::mir::Field;
 use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange};
 use rustc_middle::ty::layout::IntegerExt;
 use rustc_middle::ty::{self, Const, Ty, TyCtxt, VariantDef};
+use rustc_middle::{middle::stability::EvalResult, mir::interpret::ConstValue};
 use rustc_session::lint;
 use rustc_span::{Span, DUMMY_SP};
 use rustc_target::abi::{Integer, Size, VariantIdx};
@@ -675,6 +675,36 @@ impl<'tcx> Constructor<'tcx> {
         }
     }
 
+    /// Checks if the `Constructor` is a variant and `TyCtxt::eval_stability` returns
+    /// `EvalResult::Deny { .. }`.
+    ///
+    /// This means that the variant has a stdlib unstable feature marking it.
+    pub(super) fn is_unstable_variant(&self, pcx: PatCtxt<'_, '_, 'tcx>) -> bool {
+        if let Constructor::Variant(idx) = self {
+            if let ty::Adt(adt, _) = pcx.ty.kind() {
+                let variant_def_id = adt.variants[*idx].def_id;
+                // Filter variants that depend on a disabled unstable feature.
+                return matches!(
+                    pcx.cx.tcx.eval_stability(variant_def_id, None, DUMMY_SP, None),
+                    EvalResult::Deny { .. }
+                );
+            }
+        }
+        false
+    }
+
+    /// Checks if the `Constructor` is a `Constructor::Variant` with a `#[doc(hidden)]`
+    /// attribute.
+    pub(super) fn is_doc_hidden_variant(&self, pcx: PatCtxt<'_, '_, 'tcx>) -> bool {
+        if let Constructor::Variant(idx) = self {
+            if let ty::Adt(adt, _) = pcx.ty.kind() {
+                let variant_def_id = adt.variants[*idx].def_id;
+                return pcx.cx.tcx.is_doc_hidden(variant_def_id);
+            }
+        }
+        false
+    }
+
     fn variant_index_for_adt(&self, adt: &'tcx ty::AdtDef) -> VariantIdx {
         match *self {
             Variant(idx) => idx,
@@ -929,36 +959,33 @@ impl<'tcx> SplitWildcard<'tcx> {
                 // witness.
                 let is_declared_nonexhaustive = cx.is_foreign_non_exhaustive_enum(pcx.ty);
 
+                let is_exhaustive_pat_feature = cx.tcx.features().exhaustive_patterns;
+
                 // If `exhaustive_patterns` is disabled and our scrutinee is an empty enum, we treat it
                 // as though it had an "unknown" constructor to avoid exposing its emptiness. The
                 // exception is if the pattern is at the top level, because we want empty matches to be
                 // considered exhaustive.
-                let is_secretly_empty = def.variants.is_empty()
-                    && !cx.tcx.features().exhaustive_patterns
-                    && !pcx.is_top_level;
-
-                if is_secretly_empty {
-                    smallvec![NonExhaustive]
-                } else if is_declared_nonexhaustive {
-                    def.variants
-                        .indices()
-                        .map(|idx| Variant(idx))
-                        .chain(Some(NonExhaustive))
-                        .collect()
-                } else if cx.tcx.features().exhaustive_patterns {
-                    // If `exhaustive_patterns` is enabled, we exclude variants known to be
-                    // uninhabited.
-                    def.variants
-                        .iter_enumerated()
-                        .filter(|(_, v)| {
-                            !v.uninhabited_from(cx.tcx, substs, def.adt_kind(), cx.param_env)
-                                .contains(cx.tcx, cx.module)
-                        })
-                        .map(|(idx, _)| Variant(idx))
-                        .collect()
-                } else {
-                    def.variants.indices().map(|idx| Variant(idx)).collect()
+                let is_secretly_empty =
+                    def.variants.is_empty() && !is_exhaustive_pat_feature && !pcx.is_top_level;
+
+                let mut ctors: SmallVec<[_; 1]> = def
+                    .variants
+                    .iter_enumerated()
+                    .filter(|(_, v)| {
+                        // If `exhaustive_patterns` is enabled, we exclude variants known to be
+                        // uninhabited.
+                        let is_uninhabited = is_exhaustive_pat_feature
+                            && v.uninhabited_from(cx.tcx, substs, def.adt_kind(), cx.param_env)
+                                .contains(cx.tcx, cx.module);
+                        !is_uninhabited
+                    })
+                    .map(|(idx, _)| Variant(idx))
+                    .collect();
+
+                if is_secretly_empty || is_declared_nonexhaustive {
+                    ctors.push(NonExhaustive);
                 }
+                ctors
             }
             ty::Char => {
                 smallvec![
@@ -1068,7 +1095,7 @@ impl<'tcx> SplitWildcard<'tcx> {
                     Missing {
                         nonexhaustive_enum_missing_real_variants: self
                             .iter_missing(pcx)
-                            .any(|c| !c.is_non_exhaustive()),
+                            .any(|c| !(c.is_non_exhaustive() || c.is_unstable_variant(pcx))),
                     }
                 } else {
                     Missing { nonexhaustive_enum_missing_real_variants: false }
@@ -1222,9 +1249,9 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
 
 /// Values and patterns can be represented as a constructor applied to some fields. This represents
 /// a pattern in this form.
-/// This also keeps track of whether the pattern has been foundreachable during analysis. For this
+/// This also keeps track of whether the pattern has been found reachable during analysis. For this
 /// reason we should be careful not to clone patterns for which we care about that. Use
-/// `clone_and_forget_reachability` is you're sure.
+/// `clone_and_forget_reachability` if you're sure.
 pub(crate) struct DeconstructedPat<'p, 'tcx> {
     ctor: Constructor<'tcx>,
     fields: Fields<'p, 'tcx>,

compiler/rustc_mir_build/src/thir/pattern/usefulness.rs

@@ -585,15 +585,33 @@ impl<'p, 'tcx> Usefulness<'p, 'tcx> {
                     } else {
                         let mut split_wildcard = SplitWildcard::new(pcx);
                         split_wildcard.split(pcx, matrix.heads().map(DeconstructedPat::ctor));
+
+                        // This lets us know if we skipped any variants because they are marked
+                        // `doc(hidden)` or they are unstable feature gate (only stdlib types).
+                        let mut hide_variant_show_wild = false;
                         // Construct for each missing constructor a "wild" version of this
                         // constructor, that matches everything that can be built with
                         // it. For example, if `ctor` is a `Constructor::Variant` for
                         // `Option::Some`, we get the pattern `Some(_)`.
-                        split_wildcard
+                        let mut new: Vec<DeconstructedPat<'_, '_>> = split_wildcard
                             .iter_missing(pcx)
-                            .cloned()
-                            .map(|missing_ctor| DeconstructedPat::wild_from_ctor(pcx, missing_ctor))
-                            .collect()
+                            .filter_map(|missing_ctor| {
+                                // Check if this variant is marked `doc(hidden)`
+                                if missing_ctor.is_doc_hidden_variant(pcx)
+                                    || missing_ctor.is_unstable_variant(pcx)
+                                {
+                                    hide_variant_show_wild = true;
+                                    return None;
+                                }
+                                Some(DeconstructedPat::wild_from_ctor(pcx, missing_ctor.clone()))
+                            })
+                            .collect();
+
+                        if hide_variant_show_wild {
+                            new.push(DeconstructedPat::wildcard(pcx.ty));
+                        }
+
+                        new
                     };
 
                     witnesses
@@ -851,8 +869,10 @@ fn is_useful<'p, 'tcx>(
                     split_wildcard
                         .iter_missing(pcx)
                         // Filter out the `NonExhaustive` because we want to list only real
-                        // variants.
-                        .filter(|c| !c.is_non_exhaustive())
+                        // variants. Also remove any unstable feature gated variants.
+                        // Because of how we computed `nonexhaustive_enum_missing_real_variants`,
+                        // this will not return an empty `Vec`.
+                        .filter(|c| !(c.is_non_exhaustive() || c.is_unstable_variant(pcx)))
                         .cloned()
                         .map(|missing_ctor| DeconstructedPat::wild_from_ctor(pcx, missing_ctor))
                         .collect::<Vec<_>>()

src/test/ui/pattern/usefulness/auxiliary/hidden.rs

@@ -0,0 +1,6 @@
+pub enum Foo {
+    A,
+    B,
+    #[doc(hidden)]
+    C,
+}

src/test/ui/pattern/usefulness/auxiliary/unstable.rs

@@ -0,0 +1,12 @@
+#![feature(staged_api)]
+#![stable(feature = "stable_test_feature", since = "1.0.0")]
+
+#[stable(feature = "stable_test_feature", since = "1.0.0")]
+pub enum Foo {
+    #[stable(feature = "stable_test_feature", since = "1.0.0")]
+    Stable,
+    #[stable(feature = "stable_test_feature", since = "1.0.0")]
+    Stable2,
+    #[unstable(feature = "unstable_test_feature", issue = "none")]
+    Unstable,
+}

src/test/ui/pattern/usefulness/doc-hidden-non-exhaustive.rs

@@ -0,0 +1,30 @@
+// aux-build:hidden.rs
+
+extern crate hidden;
+
+use hidden::Foo;
+
+fn main() {
+    match Foo::A {
+        Foo::A => {}
+        Foo::B => {}
+    }
+    //~^^^^ non-exhaustive patterns: `_` not covered
+
+    match Foo::A {
+        Foo::A => {}
+        Foo::C => {}
+    }
+    //~^^^^ non-exhaustive patterns: `B` not covered
+
+    match Foo::A {
+        Foo::A => {}
+    }
+    //~^^^ non-exhaustive patterns: `B` and `_` not covered
+
+    match None {
+        None => {}
+        Some(Foo::A) => {}
+    }
+    //~^^^^ non-exhaustive patterns: `Some(B)` and `Some(_)` not covered
+}

src/test/ui/pattern/usefulness/doc-hidden-non-exhaustive.stderr

@@ -0,0 +1,54 @@
+error[E0004]: non-exhaustive patterns: `_` not covered
+  --> $DIR/doc-hidden-non-exhaustive.rs:8:11
+   |
+LL |     match Foo::A {
+   |           ^^^^^^ pattern `_` not covered
+   |
+   = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
+   = note: the matched value is of type `Foo`
+
+error[E0004]: non-exhaustive patterns: `B` not covered
+  --> $DIR/doc-hidden-non-exhaustive.rs:14:11
+   |
+LL |     match Foo::A {
+   |           ^^^^^^ pattern `B` not covered
+   |
+  ::: $DIR/auxiliary/hidden.rs:3:5
+   |
+LL |     B,
+   |     - not covered
+   |
+   = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
+   = note: the matched value is of type `Foo`
+
+error[E0004]: non-exhaustive patterns: `B` and `_` not covered
+  --> $DIR/doc-hidden-non-exhaustive.rs:20:11
+   |
+LL |     match Foo::A {
+   |           ^^^^^^ patterns `B` and `_` not covered
+   |
+  ::: $DIR/auxiliary/hidden.rs:3:5
+   |
+LL |     B,
+   |     - not covered
+   |
+   = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
+   = note: the matched value is of type `Foo`
+
+error[E0004]: non-exhaustive patterns: `Some(B)` and `Some(_)` not covered
+  --> $DIR/doc-hidden-non-exhaustive.rs:25:11
+   |
+LL |     match None {
+   |           ^^^^ patterns `Some(B)` and `Some(_)` not covered
+   |
+  ::: $SRC_DIR/core/src/option.rs:LL:COL
+   |
+LL |     Some(#[stable(feature = "rust1", since = "1.0.0")] T),
+   |     ---- not covered
+   |
+   = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
+   = note: the matched value is of type `Option<Foo>`
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0004`.

src/test/ui/pattern/usefulness/stable-gated-patterns.rs

@@ -0,0 +1,18 @@
+// aux-build:unstable.rs
+
+extern crate unstable;
+
+use unstable::Foo;
+
+fn main() {
+    match Foo::Stable {
+        Foo::Stable => {}
+    }
+    //~^^^ non-exhaustive patterns: `Stable2` and `_` not covered
+
+    match Foo::Stable {
+        Foo::Stable => {}
+        Foo::Stable2 => {}
+    }
+    //~^^^^ non-exhaustive patterns: `_` not covered
+}

src/test/ui/pattern/usefulness/stable-gated-patterns.stderr

@@ -0,0 +1,26 @@
+error[E0004]: non-exhaustive patterns: `Stable2` and `_` not covered
+  --> $DIR/stable-gated-patterns.rs:8:11
+   |
+LL |     match Foo::Stable {
+   |           ^^^^^^^^^^^ patterns `Stable2` and `_` not covered
+   |
+  ::: $DIR/auxiliary/unstable.rs:9:5
+   |
+LL |     Stable2,
+   |     ------- not covered
+   |
+   = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
+   = note: the matched value is of type `Foo`
+
+error[E0004]: non-exhaustive patterns: `_` not covered
+  --> $DIR/stable-gated-patterns.rs:13:11
+   |
+LL |     match Foo::Stable {
+   |           ^^^^^^^^^^^ pattern `_` not covered
+   |
+   = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
+   = note: the matched value is of type `Foo`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0004`.

src/test/ui/pattern/usefulness/unstable-gated-patterns.rs

@@ -0,0 +1,22 @@
+#![feature(unstable_test_feature)]
+
+// aux-build:unstable.rs
+
+extern crate unstable;
+
+use unstable::Foo;
+
+fn main() {
+    match Foo::Stable {
+        Foo::Stable => {}
+        Foo::Stable2 => {}
+    }
+    //~^^^^ non-exhaustive patterns: `Unstable` not covered
+
+    // Ok: all variants are explicitly matched
+    match Foo::Stable {
+        Foo::Stable => {}
+        Foo::Stable2 => {}
+        Foo::Unstable => {}
+    }
+}

src/test/ui/pattern/usefulness/unstable-gated-patterns.stderr

@@ -0,0 +1,17 @@
+error[E0004]: non-exhaustive patterns: `Unstable` not covered
+  --> $DIR/unstable-gated-patterns.rs:10:11
+   |
+LL |     match Foo::Stable {
+   |           ^^^^^^^^^^^ pattern `Unstable` not covered
+   |
+  ::: $DIR/auxiliary/unstable.rs:11:5
+   |
+LL |     Unstable,
+   |     -------- not covered
+   |
+   = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
+   = note: the matched value is of type `Foo`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0004`.