Skip to content

Commit 786c94a

Browse files
borsbors
committed
Auto merge of rust-lang#116734 - Nadrieril:lint-per-column, r=cjgillot
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 rust-lang#111651. cc `@eholk` who reviewed it at the time. Compared to then, I'm more confident this is the right approach.
2 parents 6f97d83 + ca869e3 commit 786c94a

11 files changed

+579
-366
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
};
@@ -279,7 +279,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
279279

280280
let scrut = &self.thir[scrut];
281281
let scrut_ty = scrut.ty;
282-
let report = compute_match_usefulness(&cx, &tarms, self.lint_level, scrut_ty);
282+
let report = compute_match_usefulness(&cx, &tarms, self.lint_level, scrut_ty, scrut.span);
283283

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

478479
// Note: we ignore whether the pattern is unreachable (i.e. whether the type is empty). We
479480
// only care about exhaustiveness here.
@@ -662,7 +663,7 @@ fn is_let_irrefutable<'p, 'tcx>(
662663
pat: &'p DeconstructedPat<'p, 'tcx>,
663664
) -> bool {
664665
let arms = [MatchArm { pat, hir_id: pat_id, has_guard: false }];
665-
let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty());
666+
let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty(), pat.span());
666667

667668
// Report if the pattern is unreachable, which can only occur when the type is uninhabited.
668669
// This also reports unreachable sub-patterns though, so we can't just replace it with an
@@ -701,8 +702,8 @@ fn report_arm_reachability<'p, 'tcx>(
701702
}
702703
}
703704

704-
fn collect_non_exhaustive_tys<'p, 'tcx>(
705-
pat: &DeconstructedPat<'p, 'tcx>,
705+
fn collect_non_exhaustive_tys<'tcx>(
706+
pat: &WitnessPat<'tcx>,
706707
non_exhaustive_tys: &mut FxHashSet<Ty<'tcx>>,
707708
) {
708709
if matches!(pat.ctor(), Constructor::NonExhaustive) {
@@ -718,7 +719,7 @@ fn non_exhaustive_match<'p, 'tcx>(
718719
thir: &Thir<'tcx>,
719720
scrut_ty: Ty<'tcx>,
720721
sp: Span,
721-
witnesses: Vec<DeconstructedPat<'p, 'tcx>>,
722+
witnesses: Vec<WitnessPat<'tcx>>,
722723
arms: &[ArmId],
723724
expr_span: Span,
724725
) -> ErrorGuaranteed {
@@ -896,10 +897,10 @@ fn non_exhaustive_match<'p, 'tcx>(
896897

897898
pub(crate) fn joined_uncovered_patterns<'p, 'tcx>(
898899
cx: &MatchCheckCtxt<'p, 'tcx>,
899-
witnesses: &[DeconstructedPat<'p, 'tcx>],
900+
witnesses: &[WitnessPat<'tcx>],
900901
) -> String {
901902
const LIMIT: usize = 3;
902-
let pat_to_str = |pat: &DeconstructedPat<'p, 'tcx>| pat.to_pat(cx).to_string();
903+
let pat_to_str = |pat: &WitnessPat<'tcx>| pat.to_pat(cx).to_string();
903904
match witnesses {
904905
[] => bug!(),
905906
[witness] => format!("`{}`", witness.to_pat(cx)),
@@ -916,7 +917,7 @@ pub(crate) fn joined_uncovered_patterns<'p, 'tcx>(
916917
}
917918

918919
pub(crate) fn pattern_not_covered_label(
919-
witnesses: &[DeconstructedPat<'_, '_>],
920+
witnesses: &[WitnessPat<'_>],
920921
joined_patterns: &str,
921922
) -> String {
922923
format!("pattern{} {} not covered", rustc_errors::pluralize!(witnesses.len()), joined_patterns)
@@ -927,7 +928,7 @@ fn adt_defined_here<'p, 'tcx>(
927928
cx: &MatchCheckCtxt<'p, 'tcx>,
928929
err: &mut Diagnostic,
929930
ty: Ty<'tcx>,
930-
witnesses: &[DeconstructedPat<'p, 'tcx>],
931+
witnesses: &[WitnessPat<'tcx>],
931932
) {
932933
let ty = ty.peel_refs();
933934
if let ty::Adt(def, _) = ty.kind() {
@@ -958,7 +959,7 @@ fn adt_defined_here<'p, 'tcx>(
958959
fn maybe_point_at_variant<'a, 'p: 'a, 'tcx: 'a>(
959960
cx: &MatchCheckCtxt<'p, 'tcx>,
960961
def: AdtDef<'tcx>,
961-
patterns: impl Iterator<Item = &'a DeconstructedPat<'p, 'tcx>>,
962+
patterns: impl Iterator<Item = &'a WitnessPat<'tcx>>,
962963
) -> Vec<Span> {
963964
use Constructor::*;
964965
let mut covered = vec![];

0 commit comments

Comments
 (0)