Skip to content

Commit

Permalink
Lint non_exhaustive_omitted_patterns per column
Browse files Browse the repository at this point in the history
  • Loading branch information
Nadrieril committed Oct 14, 2023
1 parent 29bd8be commit 33a65f2
Show file tree
Hide file tree
Showing 10 changed files with 279 additions and 232 deletions.
34 changes: 21 additions & 13 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3993,8 +3993,13 @@ declare_lint! {
}

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.
/// The `non_exhaustive_omitted_patterns` lint aims to help consumers of a `#[non_exhaustive]`
/// struct or enum who want to match all of its fields/variants explicitly.
///
/// The `#[non_exhaustive]` annotation forces matches to use wildcards, so exhaustiveness
/// checking cannot be used to ensure that all fields/variants are matched explicitly. To remedy
/// this, this allow-by-default lint warns the user when a match mentions some but not all of
/// the fields/variants of a `#[non_exhaustive]` struct or enum.
///
/// ### Example
///
Expand All @@ -4008,39 +4013,42 @@ declare_lint! {
///
/// // in crate B
/// #![feature(non_exhaustive_omitted_patterns_lint)]
/// #[warn(non_exhaustive_omitted_patterns)]
/// match Bar::A {
/// Bar::A => {},
/// #[warn(non_exhaustive_omitted_patterns)]
/// _ => {},
/// }
/// ```
///
/// This will produce:
///
/// ```text
/// warning: reachable patterns not covered of non exhaustive enum
/// warning: some variants are not matched explicitly
/// --> $DIR/reachable-patterns.rs:70:9
/// |
/// LL | _ => {}
/// | ^ pattern `B` not covered
/// LL | match Bar::A {
/// | ^ pattern `Bar::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
/// = help: ensure that all variants are matched explicitly by adding the suggested match arms
/// = note: the matched value is of type `Bar` and the `non_exhaustive_omitted_patterns` attribute was found
/// ```
///
/// Warning: setting this to `deny` will make upstream non-breaking changes (adding fields or
/// variants to a `#[non_exhaustive]` struct or enum) break your crate. This goes against
/// expected semver behavior.
///
/// ### 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.
/// 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",
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {

let scrut = &self.thir[scrut];
let scrut_ty = scrut.ty;
let report = compute_match_usefulness(&cx, &tarms, self.lint_level, scrut_ty);
let report = compute_match_usefulness(&cx, &tarms, self.lint_level, scrut_ty, scrut.span);

match source {
// Don't report arm reachability of desugared `match $iter.into_iter() { iter => .. }`
Expand Down Expand Up @@ -431,7 +431,8 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
let pattern = self.lower_pattern(&mut cx, pat);
let pattern_ty = pattern.ty();
let arm = MatchArm { pat: pattern, hir_id: self.lint_level, has_guard: false };
let report = compute_match_usefulness(&cx, &[arm], self.lint_level, pattern_ty);
let report =
compute_match_usefulness(&cx, &[arm], self.lint_level, pattern_ty, pattern.span());

// Note: we ignore whether the pattern is unreachable (i.e. whether the type is empty). We
// only care about exhaustiveness here.
Expand Down Expand Up @@ -622,7 +623,7 @@ fn is_let_irrefutable<'p, 'tcx>(
pat: &'p DeconstructedPat<'p, 'tcx>,
) -> bool {
let arms = [MatchArm { pat, hir_id: pat_id, has_guard: false }];
let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty());
let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty(), pat.span());

// Report if the pattern is unreachable, which can only occur when the type is uninhabited.
// This also reports unreachable sub-patterns though, so we can't just replace it with an
Expand Down
46 changes: 18 additions & 28 deletions compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,18 +629,11 @@ pub(super) enum Constructor<'tcx> {
/// `#[doc(hidden)]` ones.
Hidden,
/// Fake extra constructor for constructors that are not seen in the matrix, as explained in the
/// code for [`Constructor::split`]. The carried `bool` is used for the
/// `non_exhaustive_omitted_patterns` lint.
Missing {
nonexhaustive_enum_missing_visible_variants: bool,
},
/// code for [`Constructor::split`].
Missing,
}

impl<'tcx> Constructor<'tcx> {
pub(super) fn is_wildcard(&self) -> bool {
matches!(self, Wildcard)
}

pub(super) fn is_non_exhaustive(&self) -> bool {
matches!(self, NonExhaustive)
}
Expand Down Expand Up @@ -778,14 +771,8 @@ impl<'tcx> Constructor<'tcx> {
let all_missing = split_set.present.is_empty();
let report_when_all_missing =
pcx.is_top_level && !IntRange::is_integral(pcx.ty);
let ctor = if all_missing && !report_when_all_missing {
Wildcard
} else {
Missing {
nonexhaustive_enum_missing_visible_variants: split_set
.nonexhaustive_enum_missing_visible_variants,
}
};
let ctor =
if all_missing && !report_when_all_missing { Wildcard } else { Missing };
smallvec![ctor]
} else {
split_set.present
Expand Down Expand Up @@ -905,11 +892,9 @@ pub(super) enum ConstructorSet {
/// either fully included in or disjoint from each constructor in the column. This avoids
/// non-trivial intersections like between `0..10` and `5..15`.
#[derive(Debug)]
struct SplitConstructorSet<'tcx> {
present: SmallVec<[Constructor<'tcx>; 1]>,
missing: Vec<Constructor<'tcx>>,
/// For the `non_exhaustive_omitted_patterns` lint.
nonexhaustive_enum_missing_visible_variants: bool,
pub(super) struct SplitConstructorSet<'tcx> {
pub(super) present: SmallVec<[Constructor<'tcx>; 1]>,
pub(super) missing: Vec<Constructor<'tcx>>,
}

impl ConstructorSet {
Expand Down Expand Up @@ -1039,7 +1024,7 @@ impl ConstructorSet {
/// constructors to 1/ determine which constructors of the type (if any) are missing; 2/ split
/// constructors to handle non-trivial intersections e.g. on ranges or slices.
#[instrument(level = "debug", skip(self, pcx, ctors), ret)]
fn split<'a, 'tcx>(
pub(super) fn split<'a, 'tcx>(
&self,
pcx: &PatCtxt<'_, '_, 'tcx>,
ctors: impl Iterator<Item = &'a Constructor<'tcx>> + Clone,
Expand All @@ -1051,7 +1036,6 @@ impl ConstructorSet {
let mut missing = Vec::new();
// Constructors in `ctors`, except wildcards.
let mut seen = ctors.filter(|c| !(matches!(c, Opaque | Wildcard)));
let mut nonexhaustive_enum_missing_visible_variants = false;
match self {
ConstructorSet::Single => {
if seen.next().is_none() {
Expand All @@ -1063,6 +1047,7 @@ impl ConstructorSet {
ConstructorSet::Variants { visible_variants, hidden_variants, non_exhaustive } => {
let seen_set: FxHashSet<_> = seen.map(|c| c.as_variant().unwrap()).collect();
let mut skipped_a_hidden_variant = false;

for variant in visible_variants {
let ctor = Variant(*variant);
if seen_set.contains(&variant) {
Expand All @@ -1071,8 +1056,6 @@ impl ConstructorSet {
missing.push(ctor);
}
}
nonexhaustive_enum_missing_visible_variants =
*non_exhaustive && !missing.is_empty();

for variant in hidden_variants {
let ctor = Variant(*variant);
Expand Down Expand Up @@ -1159,7 +1142,7 @@ impl ConstructorSet {
ConstructorSet::Uninhabited => {}
}

SplitConstructorSet { present, missing, nonexhaustive_enum_missing_visible_variants }
SplitConstructorSet { present, missing }
}

/// Compute the set of constructors missing from this column.
Expand Down Expand Up @@ -1519,6 +1502,13 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
pub(super) fn is_or_pat(&self) -> bool {
matches!(self.ctor, Or)
}
pub(super) fn flatten_or_pat(&'p self) -> SmallVec<[&'p Self; 1]> {
if self.is_or_pat() {
self.iter_fields().flat_map(|p| p.flatten_or_pat()).collect()
} else {
smallvec![self]
}
}

pub(super) fn ctor(&self) -> &Constructor<'tcx> {
&self.ctor
Expand Down Expand Up @@ -1704,7 +1694,7 @@ impl<'p, 'tcx> fmt::Debug for DeconstructedPat<'p, 'tcx> {
#[derive(Debug, Clone)]
pub(crate) struct WitnessPat<'tcx> {
ctor: Constructor<'tcx>,
fields: Vec<WitnessPat<'tcx>>,
pub(crate) fields: Vec<WitnessPat<'tcx>>,
ty: Ty<'tcx>,
}

Expand Down
Loading

0 comments on commit 33a65f2

Please sign in to comment.