Skip to content

Commit

Permalink
Filter unstable and doc hidden variants in usefulness checking
Browse files Browse the repository at this point in the history
Add test cases for unstable variants
Add test cases for doc hidden variants
Move is_doc_hidden to method on TyCtxt
Add unstable variants test to reachable-patterns ui test
Rename reachable-patterns -> omitted-patterns
  • Loading branch information
DevinR528 committed Oct 12, 2021
1 parent 02f2b31 commit 2a042d6
Show file tree
Hide file tree
Showing 16 changed files with 428 additions and 92 deletions.
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.
// 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

0 comments on commit 2a042d6

Please sign in to comment.