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

Rewrite non_exhaustive_omitted_patterns as a separate pass #111651

Closed
wants to merge 3 commits into from
Closed
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
23 changes: 12 additions & 11 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3798,8 +3798,10 @@ 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 detects when some variants or fields of a
/// `#[non_exhaustive]` struct or enum are not mentioned explicitly in a pattern. This allows
/// downstream crates to be warned when new variants or fields are added to the upstream struct
/// or enum.
eholk marked this conversation as resolved.
Show resolved Hide resolved
///
/// ### Example
///
Expand All @@ -3814,9 +3816,9 @@ 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)]
eholk marked this conversation as resolved.
Show resolved Hide resolved
/// _ => {},
/// }
/// ```
Expand All @@ -3827,8 +3829,8 @@ declare_lint! {
/// warning: reachable patterns not covered of non exhaustive enum
/// --> $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
Expand All @@ -3841,12 +3843,11 @@ declare_lint! {
///
/// ### 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. 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 @@ -250,7 +250,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 @@ -415,7 +415,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 @@ -583,7 +584,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: 24 additions & 22 deletions compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -643,9 +643,8 @@ 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`]. The carried `bool` is used for the `non_exhaustive_omitted_patterns`
/// lint.
Missing { nonexhaustive_enum_missing_real_variants: bool },
/// for [`SplitWildcard`].
Missing,
/// Wildcard pattern.
Wildcard,
/// Or-pattern.
Expand Down Expand Up @@ -1053,6 +1052,16 @@ impl<'tcx> SplitWildcard<'tcx> {
self.all_ctors.iter().filter(move |ctor| !ctor.is_covered_by_any(pcx, &self.matrix_ctors))
}

/// Iterate over the constructors for this type that are present in the matrix. This has the
/// effect of deduplicating present constructors.
/// WARNING: this omits special constructors like `Wildcard` and `Opaque`.
pub(super) fn iter_present<'a, 'p>(
&'a self,
pcx: &'a PatCtxt<'a, 'p, 'tcx>,
) -> impl Iterator<Item = &'a Constructor<'tcx>> + Captures<'p> {
self.all_ctors.iter().filter(move |ctor| ctor.is_covered_by_any(pcx, &self.matrix_ctors))
}

/// Return the set of constructors resulting from splitting the wildcard. As explained at the
/// top of the file, if any constructors are missing we can ignore the present ones.
fn into_ctors(self, pcx: &PatCtxt<'_, '_, 'tcx>) -> SmallVec<[Constructor<'tcx>; 1]> {
Expand Down Expand Up @@ -1086,15 +1095,7 @@ 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 {
if pcx.is_non_exhaustive {
Missing {
nonexhaustive_enum_missing_real_variants: self
.iter_missing(pcx)
.any(|c| !(c.is_non_exhaustive() || c.is_unstable_variant(pcx))),
}
} else {
Missing { nonexhaustive_enum_missing_real_variants: false }
}
Missing
} else {
Wildcard
};
Expand Down Expand Up @@ -1240,9 +1241,10 @@ 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 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` if you're sure.
/// This also uses interior mutability to keep 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.
#[derive(Clone)]
pub(crate) struct DeconstructedPat<'p, 'tcx> {
ctor: Constructor<'tcx>,
fields: Fields<'p, 'tcx>,
Expand Down Expand Up @@ -1273,12 +1275,6 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
DeconstructedPat::new(ctor, fields, pcx.ty, pcx.span)
}

/// Clone this value. This method emphasizes that cloning loses reachability information and
/// should be done carefully.
pub(super) fn clone_and_forget_reachability(&self) -> Self {
DeconstructedPat::new(self.ctor.clone(), self.fields, self.ty, self.span)
}

pub(crate) fn from_pat(cx: &MatchCheckCtxt<'p, 'tcx>, pat: &Pat<'tcx>) -> Self {
let mkpat = |pat| DeconstructedPat::from_pat(cx, pat);
let ctor;
Expand Down Expand Up @@ -1522,6 +1518,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 @@ -1596,7 +1599,6 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
self.collect_unreachable_spans(&mut spans);
spans
}

fn collect_unreachable_spans(&self, spans: &mut Vec<Span>) {
// We don't look at subpatterns if we already reported the whole pattern as unreachable.
if !self.is_reachable() {
Expand Down
Loading