Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: non_exhaustive_omitted_patterns by filtering unstable and doc hidden variants #89105

Merged
merged 1 commit into from
Oct 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
85 changes: 56 additions & 29 deletions compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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![
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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>,
Expand Down
32 changes: 26 additions & 6 deletions compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Nadrieril marked this conversation as resolved.
Show resolved Hide resolved
// 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<_>>()
Expand Down
6 changes: 6 additions & 0 deletions src/test/ui/pattern/usefulness/auxiliary/hidden.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pub enum Foo {
A,
B,
#[doc(hidden)]
C,
}
12 changes: 12 additions & 0 deletions src/test/ui/pattern/usefulness/auxiliary/unstable.rs
Original file line number Diff line number Diff line change
@@ -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,
}
30 changes: 30 additions & 0 deletions src/test/ui/pattern/usefulness/doc-hidden-non-exhaustive.rs
Original file line number Diff line number Diff line change
@@ -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
}
54 changes: 54 additions & 0 deletions src/test/ui/pattern/usefulness/doc-hidden-non-exhaustive.stderr
Original file line number Diff line number Diff line change
@@ -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`.
18 changes: 18 additions & 0 deletions src/test/ui/pattern/usefulness/stable-gated-patterns.rs
Original file line number Diff line number Diff line change
@@ -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
}
26 changes: 26 additions & 0 deletions src/test/ui/pattern/usefulness/stable-gated-patterns.stderr
Original file line number Diff line number Diff line change
@@ -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`.
22 changes: 22 additions & 0 deletions src/test/ui/pattern/usefulness/unstable-gated-patterns.rs
Original file line number Diff line number Diff line change
@@ -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 => {}
}
}
17 changes: 17 additions & 0 deletions src/test/ui/pattern/usefulness/unstable-gated-patterns.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading