Skip to content

Commit

Permalink
Auto merge of #86809 - DevinR528:reachable-pat, r=Nadrieril
Browse files Browse the repository at this point in the history
Add non_exhaustive_omitted_patterns lint related to rfc-2008-non_exhaustive

Fixes: #84332

This PR adds `non_exhaustive_omitted_patterns`, an allow by default lint that is triggered when a `non_exhaustive` type is missing explicit patterns. The warning or deny attribute can be put above the wildcard `_` pattern on enums or on the expression for enums or structs. The lint is capable of warning about multiple types within the same pattern. This lint will not be triggered for `if let ..` patterns.

```rust
// crate A
#[non_exhaustive]
pub struct Foo {
    a: u8,
    b: usize,
}
#[non_exhaustive]
pub enum Bar {
    A(Foo),
    B,
}

// crate B
#[deny(non_exhaustive_omitted_patterns)] // here
match Bar::B {
    Bar::B => {}
    #[deny(non_exhaustive_omitted_patterns)] // or here
    _ => {}
}

#[warn(non_exhaustive_omitted_patterns)] // only here
let Foo { a, .. } = Foo::default();

#[deny(non_exhaustive_omitted_patterns)]
match Bar::B {
    // triggers for Bar::B, and Foo.b
    Bar::A(Foo { a, .. }) => {}
    // if the attribute was here only Bar::B would cause a warning
    _ => {}
}
```
  • Loading branch information
bors committed Sep 16, 2021
2 parents 34327f6 + 33a06b7 commit 2b5ddf3
Show file tree
Hide file tree
Showing 10 changed files with 626 additions and 68 deletions.
54 changes: 54 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3010,6 +3010,7 @@ declare_lint_pass! {
UNSUPPORTED_CALLING_CONVENTIONS,
BREAK_WITH_LABEL_AND_LOOP,
UNUSED_ATTRIBUTES,
NON_EXHAUSTIVE_OMITTED_PATTERNS,
]
}

Expand Down Expand Up @@ -3416,3 +3417,56 @@ declare_lint! {
Warn,
"`break` expression with label and unlabeled loop as value expression"
}

declare_lint! {
/// The `non_exhaustive_omitted_patterns` lint detects when a wildcard (`_` or `..`) in a
/// pattern for a `#[non_exhaustive]` struct or enum is reachable.
///
/// ### Example
///
/// ```rust,ignore (needs separate crate)
/// // crate A
/// #[non_exhaustive]
/// pub enum Bar {
/// A,
/// B, // added variant in non breaking change
/// }
///
/// // in crate B
/// match Bar::A {
/// Bar::A => {},
/// #[warn(non_exhaustive_omitted_patterns)]
/// _ => {},
/// }
/// ```
///
/// This will produce:
///
/// ```text
/// warning: reachable patterns not covered of non exhaustive enum
/// --> $DIR/reachable-patterns.rs:70:9
/// |
/// LL | _ => {}
/// | ^ pattern `B` not covered
/// |
/// note: the lint level is defined here
/// --> $DIR/reachable-patterns.rs:69:16
/// |
/// LL | #[warn(non_exhaustive_omitted_patterns)]
/// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/// = help: ensure that all possible cases are being handled by adding the suggested match arms
/// = note: the matched value is of type `Bar` and the `non_exhaustive_omitted_patterns` attribute was found
/// ```
///
/// ### Explanation
///
/// Structs and enums tagged with `#[non_exhaustive]` force the user to add a
/// (potentially redundant) wildcard when pattern-matching, to allow for future
/// addition of fields or variants. The `non_exhaustive_omitted_patterns` lint
/// detects when such a wildcard happens to actually catch some fields/variants.
/// In other words, when the match without the wildcard would not be exhaustive.
/// This lets the user be informed if new fields/variants were added.
pub NON_EXHAUSTIVE_OMITTED_PATTERNS,
Allow,
"detect when patterns of types marked `non_exhaustive` are missed",
}
9 changes: 5 additions & 4 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::{HirId, Pat};
use rustc_middle::thir::PatKind;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::lint::builtin::BINDINGS_WITH_VARIANT_NAME;
use rustc_session::lint::builtin::{IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS};
use rustc_session::lint::builtin::{
BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
};
use rustc_session::Session;
use rustc_span::{DesugaringKind, ExpnKind, Span};
use std::slice;
Expand Down Expand Up @@ -559,7 +560,7 @@ fn non_exhaustive_match<'p, 'tcx>(
err.emit();
}

fn joined_uncovered_patterns(witnesses: &[super::Pat<'_>]) -> String {
crate fn joined_uncovered_patterns(witnesses: &[super::Pat<'_>]) -> String {
const LIMIT: usize = 3;
match witnesses {
[] => bug!(),
Expand All @@ -576,7 +577,7 @@ fn joined_uncovered_patterns(witnesses: &[super::Pat<'_>]) -> String {
}
}

fn pattern_not_covered_label(witnesses: &[super::Pat<'_>], joined_patterns: &str) -> String {
crate fn pattern_not_covered_label(witnesses: &[super::Pat<'_>], joined_patterns: &str) -> String {
format!("pattern{} {} not covered", rustc_errors::pluralize!(witnesses.len()), joined_patterns)
}

Expand Down
58 changes: 44 additions & 14 deletions compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,8 +606,9 @@ pub(super) enum Constructor<'tcx> {
/// for those types for which we cannot list constructors explicitly, like `f64` and `str`.
NonExhaustive,
/// Stands for constructors that are not seen in the matrix, as explained in the documentation
/// for [`SplitWildcard`].
Missing,
/// for [`SplitWildcard`]. The carried `bool` is used for the `non_exhaustive_omitted_patterns`
/// lint.
Missing { nonexhaustive_enum_missing_real_variants: bool },
/// Wildcard pattern.
Wildcard,
}
Expand All @@ -617,6 +618,10 @@ impl<'tcx> Constructor<'tcx> {
matches!(self, Wildcard)
}

pub(super) fn is_non_exhaustive(&self) -> bool {
matches!(self, NonExhaustive)
}

fn as_int_range(&self) -> Option<&IntRange> {
match self {
IntRange(range) => Some(range),
Expand Down Expand Up @@ -756,7 +761,7 @@ impl<'tcx> Constructor<'tcx> {
// Wildcards cover anything
(_, Wildcard) => true,
// The missing ctors are not covered by anything in the matrix except wildcards.
(Missing | Wildcard, _) => false,
(Missing { .. } | Wildcard, _) => false,

(Single, Single) => true,
(Variant(self_id), Variant(other_id)) => self_id == other_id,
Expand Down Expand Up @@ -829,7 +834,7 @@ impl<'tcx> Constructor<'tcx> {
.any(|other| slice.is_covered_by(other)),
// This constructor is never covered by anything else
NonExhaustive => false,
Str(..) | FloatRange(..) | Opaque | Missing | Wildcard => {
Str(..) | FloatRange(..) | Opaque | Missing { .. } | Wildcard => {
span_bug!(pcx.span, "found unexpected ctor in all_ctors: {:?}", self)
}
}
Expand Down Expand Up @@ -919,8 +924,14 @@ impl<'tcx> SplitWildcard<'tcx> {
&& !cx.tcx.features().exhaustive_patterns
&& !pcx.is_top_level;

if is_secretly_empty || is_declared_nonexhaustive {
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.
Expand Down Expand Up @@ -975,6 +986,7 @@ impl<'tcx> SplitWildcard<'tcx> {
// This type is one for which we cannot list constructors, like `str` or `f64`.
_ => smallvec![NonExhaustive],
};

SplitWildcard { matrix_ctors: Vec::new(), all_ctors }
}

Expand Down Expand Up @@ -1039,7 +1051,17 @@ impl<'tcx> SplitWildcard<'tcx> {
// sometimes prefer reporting the list of constructors instead of just `_`.
let report_when_all_missing = pcx.is_top_level && !IntRange::is_integral(pcx.ty);
let ctor = if !self.matrix_ctors.is_empty() || report_when_all_missing {
Missing
if pcx.is_non_exhaustive {
Missing {
nonexhaustive_enum_missing_real_variants: self
.iter_missing(pcx)
.filter(|c| !c.is_non_exhaustive())
.next()
.is_some(),
}
} else {
Missing { nonexhaustive_enum_missing_real_variants: false }
}
} else {
Wildcard
};
Expand Down Expand Up @@ -1176,7 +1198,12 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
}
_ => bug!("bad slice pattern {:?} {:?}", constructor, ty),
},
Str(..) | FloatRange(..) | IntRange(..) | NonExhaustive | Opaque | Missing
Str(..)
| FloatRange(..)
| IntRange(..)
| NonExhaustive
| Opaque
| Missing { .. }
| Wildcard => Fields::Slice(&[]),
};
debug!("Fields::wildcards({:?}, {:?}) = {:#?}", constructor, ty, ret);
Expand All @@ -1189,15 +1216,18 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
/// This is roughly the inverse of `specialize_constructor`.
///
/// Examples:
/// `ctor`: `Constructor::Single`
/// `ty`: `Foo(u32, u32, u32)`
/// `self`: `[10, 20, _]`
///
/// ```text
/// ctor: `Constructor::Single`
/// ty: `Foo(u32, u32, u32)`
/// self: `[10, 20, _]`
/// returns `Foo(10, 20, _)`
///
/// `ctor`: `Constructor::Variant(Option::Some)`
/// `ty`: `Option<bool>`
/// `self`: `[false]`
/// ctor: `Constructor::Variant(Option::Some)`
/// ty: `Option<bool>`
/// self: `[false]`
/// returns `Some(false)`
/// ```
pub(super) fn apply(self, pcx: PatCtxt<'_, 'p, 'tcx>, ctor: &Constructor<'tcx>) -> Pat<'tcx> {
let subpatterns_and_indices = self.patterns_and_indices();
let mut subpatterns = subpatterns_and_indices.iter().map(|&(_, p)| p).cloned();
Expand Down Expand Up @@ -1265,7 +1295,7 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
NonExhaustive => PatKind::Wild,
Wildcard => return Pat::wildcard_from_ty(pcx.ty),
Opaque => bug!("we should not try to apply an opaque constructor"),
Missing => bug!(
Missing { .. } => bug!(
"trying to apply the `Missing` constructor; this should have been done in `apply_constructors`"
),
};
Expand Down
Loading

0 comments on commit 2b5ddf3

Please sign in to comment.