Skip to content

Commit 81a52d7

Browse files
borsbors
committed
Auto merge of #116734 - Nadrieril:lint-per-column, r=<try>
Lint `non_exhaustive_omitted_patterns` by columns This is a rework of the `non_exhaustive_omitted_patterns` lint to make it more consistent. The intent of the lint is to help consumers of `non_exhaustive` enums ensure they stay up-to-date with all upstream variants. This rewrite fixes two cases we didn't handle well before: First, because of details of exhaustiveness checking, the following wouldn't lint `Enum::C` as missing: ```rust match Some(x) { Some(Enum::A) => {} Some(Enum::B) => {} _ => {} } ``` Second, because of the fundamental workings of exhaustiveness checking, the following would treat the `true` and `false` cases separately and thus lint about missing variants: ```rust match (true, x) { (true, Enum::A) => {} (true, Enum::B) => {} (false, Enum::C) => {} _ => {} } ``` Moreover, it would correctly not lint in the case where the pair is flipped, because of asymmetry in how exhaustiveness checking proceeds. A drawback is that it no longer makes sense to set the lint level per-arm. This will silently break the lint for current users of it (but it's behind a feature gate so that's ok). The new approach is now independent of the exhaustiveness algorithm; it's a separate pass that looks at patterns column by column. This is another of the motivations for this: I'm glad to move it out of the algorithm, it was akward there. This PR is almost identical to #111651. cc `@eholk` who reviewed it at the time. Compared to then, I'm more confident this is the right approach.
2 parents 495c5dd + 33a65f2 commit 81a52d7

11 files changed

+578
-365
lines changed

‎compiler/rustc_lint_defs/src/builtin.rs

+21-13
Original file line numberDiff line numberDiff line change
@@ -3953,8 +3953,13 @@ declare_lint! {
39533953
}
39543954

39553955
declare_lint! {
3956-
/// The `non_exhaustive_omitted_patterns` lint detects when a wildcard (`_` or `..`) in a
3957-
/// pattern for a `#[non_exhaustive]` struct or enum is reachable.
3956+
/// The `non_exhaustive_omitted_patterns` lint aims to help consumers of a `#[non_exhaustive]`
3957+
/// struct or enum who want to match all of its fields/variants explicitly.
3958+
///
3959+
/// The `#[non_exhaustive]` annotation forces matches to use wildcards, so exhaustiveness
3960+
/// checking cannot be used to ensure that all fields/variants are matched explicitly. To remedy
3961+
/// this, this allow-by-default lint warns the user when a match mentions some but not all of
3962+
/// the fields/variants of a `#[non_exhaustive]` struct or enum.
39583963
///
39593964
/// ### Example
39603965
///
@@ -3968,39 +3973,42 @@ declare_lint! {
39683973
///
39693974
/// // in crate B
39703975
/// #![feature(non_exhaustive_omitted_patterns_lint)]
3976+
/// #[warn(non_exhaustive_omitted_patterns)]
39713977
/// match Bar::A {
39723978
/// Bar::A => {},
3973-
/// #[warn(non_exhaustive_omitted_patterns)]
39743979
/// _ => {},
39753980
/// }
39763981
/// ```
39773982
///
39783983
/// This will produce:
39793984
///
39803985
/// ```text
3981-
/// warning: reachable patterns not covered of non exhaustive enum
3986+
/// warning: some variants are not matched explicitly
39823987
/// --> $DIR/reachable-patterns.rs:70:9
39833988
/// |
3984-
/// LL | _ => {}
3985-
/// | ^ pattern `B` not covered
3989+
/// LL | match Bar::A {
3990+
/// | ^ pattern `Bar::B` not covered
39863991
/// |
39873992
/// note: the lint level is defined here
39883993
/// --> $DIR/reachable-patterns.rs:69:16
39893994
/// |
39903995
/// LL | #[warn(non_exhaustive_omitted_patterns)]
39913996
/// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3992-
/// = help: ensure that all possible cases are being handled by adding the suggested match arms
3997+
/// = help: ensure that all variants are matched explicitly by adding the suggested match arms
39933998
/// = note: the matched value is of type `Bar` and the `non_exhaustive_omitted_patterns` attribute was found
39943999
/// ```
39954000
///
4001+
/// Warning: setting this to `deny` will make upstream non-breaking changes (adding fields or
4002+
/// variants to a `#[non_exhaustive]` struct or enum) break your crate. This goes against
4003+
/// expected semver behavior.
4004+
///
39964005
/// ### Explanation
39974006
///
3998-
/// Structs and enums tagged with `#[non_exhaustive]` force the user to add a
3999-
/// (potentially redundant) wildcard when pattern-matching, to allow for future
4000-
/// addition of fields or variants. The `non_exhaustive_omitted_patterns` lint
4001-
/// detects when such a wildcard happens to actually catch some fields/variants.
4002-
/// In other words, when the match without the wildcard would not be exhaustive.
4003-
/// This lets the user be informed if new fields/variants were added.
4007+
/// Structs and enums tagged with `#[non_exhaustive]` force the user to add a (potentially
4008+
/// redundant) wildcard when pattern-matching, to allow for future addition of fields or
4009+
/// variants. The `non_exhaustive_omitted_patterns` lint detects when such a wildcard happens to
4010+
/// actually catch some fields/variants. In other words, when the match without the wildcard
4011+
/// would not be exhaustive. This lets the user be informed if new fields/variants were added.
40044012
pub NON_EXHAUSTIVE_OMITTED_PATTERNS,
40054013
Allow,
40064014
"detect when patterns of types marked `non_exhaustive` are missed",

‎compiler/rustc_mir_build/src/errors.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{
22
fluent_generated as fluent,
3-
thir::pattern::{deconstruct_pat::DeconstructedPat, MatchCheckCtxt},
3+
thir::pattern::{deconstruct_pat::WitnessPat, MatchCheckCtxt},
44
};
55
use rustc_errors::{
66
error_code, AddToDiagnostic, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed,
@@ -810,7 +810,7 @@ impl<'tcx> Uncovered<'tcx> {
810810
pub fn new<'p>(
811811
span: Span,
812812
cx: &MatchCheckCtxt<'p, 'tcx>,
813-
witnesses: Vec<DeconstructedPat<'p, 'tcx>>,
813+
witnesses: Vec<WitnessPat<'tcx>>,
814814
) -> Self {
815815
let witness_1 = witnesses.get(0).unwrap().to_pat(cx);
816816
Self {

‎compiler/rustc_mir_build/src/thir/pattern/check_match.rs

+13-12
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::deconstruct_pat::{Constructor, DeconstructedPat};
1+
use super::deconstruct_pat::{Constructor, DeconstructedPat, WitnessPat};
22
use super::usefulness::{
33
compute_match_usefulness, MatchArm, MatchCheckCtxt, Reachability, UsefulnessReport,
44
};
@@ -269,7 +269,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
269269

270270
let scrut = &self.thir[scrut];
271271
let scrut_ty = scrut.ty;
272-
let report = compute_match_usefulness(&cx, &tarms, self.lint_level, scrut_ty);
272+
let report = compute_match_usefulness(&cx, &tarms, self.lint_level, scrut_ty, scrut.span);
273273

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

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

627628
// Report if the pattern is unreachable, which can only occur when the type is uninhabited.
628629
// This also reports unreachable sub-patterns though, so we can't just replace it with an
@@ -661,8 +662,8 @@ fn report_arm_reachability<'p, 'tcx>(
661662
}
662663
}
663664

664-
fn collect_non_exhaustive_tys<'p, 'tcx>(
665-
pat: &DeconstructedPat<'p, 'tcx>,
665+
fn collect_non_exhaustive_tys<'tcx>(
666+
pat: &WitnessPat<'tcx>,
666667
non_exhaustive_tys: &mut FxHashSet<Ty<'tcx>>,
667668
) {
668669
if matches!(pat.ctor(), Constructor::NonExhaustive) {
@@ -678,7 +679,7 @@ fn non_exhaustive_match<'p, 'tcx>(
678679
thir: &Thir<'tcx>,
679680
scrut_ty: Ty<'tcx>,
680681
sp: Span,
681-
witnesses: Vec<DeconstructedPat<'p, 'tcx>>,
682+
witnesses: Vec<WitnessPat<'tcx>>,
682683
arms: &[ArmId],
683684
expr_span: Span,
684685
) -> ErrorGuaranteed {
@@ -860,10 +861,10 @@ fn non_exhaustive_match<'p, 'tcx>(
860861

861862
pub(crate) fn joined_uncovered_patterns<'p, 'tcx>(
862863
cx: &MatchCheckCtxt<'p, 'tcx>,
863-
witnesses: &[DeconstructedPat<'p, 'tcx>],
864+
witnesses: &[WitnessPat<'tcx>],
864865
) -> String {
865866
const LIMIT: usize = 3;
866-
let pat_to_str = |pat: &DeconstructedPat<'p, 'tcx>| pat.to_pat(cx).to_string();
867+
let pat_to_str = |pat: &WitnessPat<'tcx>| pat.to_pat(cx).to_string();
867868
match witnesses {
868869
[] => bug!(),
869870
[witness] => format!("`{}`", witness.to_pat(cx)),
@@ -880,7 +881,7 @@ pub(crate) fn joined_uncovered_patterns<'p, 'tcx>(
880881
}
881882

882883
pub(crate) fn pattern_not_covered_label(
883-
witnesses: &[DeconstructedPat<'_, '_>],
884+
witnesses: &[WitnessPat<'_>],
884885
joined_patterns: &str,
885886
) -> String {
886887
format!("pattern{} {} not covered", rustc_errors::pluralize!(witnesses.len()), joined_patterns)
@@ -891,7 +892,7 @@ fn adt_defined_here<'p, 'tcx>(
891892
cx: &MatchCheckCtxt<'p, 'tcx>,
892893
err: &mut Diagnostic,
893894
ty: Ty<'tcx>,
894-
witnesses: &[DeconstructedPat<'p, 'tcx>],
895+
witnesses: &[WitnessPat<'tcx>],
895896
) {
896897
let ty = ty.peel_refs();
897898
if let ty::Adt(def, _) = ty.kind() {
@@ -922,7 +923,7 @@ fn adt_defined_here<'p, 'tcx>(
922923
fn maybe_point_at_variant<'a, 'p: 'a, 'tcx: 'a>(
923924
cx: &MatchCheckCtxt<'p, 'tcx>,
924925
def: AdtDef<'tcx>,
925-
patterns: impl Iterator<Item = &'a DeconstructedPat<'p, 'tcx>>,
926+
patterns: impl Iterator<Item = &'a WitnessPat<'tcx>>,
926927
) -> Vec<Span> {
927928
use Constructor::*;
928929
let mut covered = vec![];

0 commit comments

Comments
 (0)