Skip to content

Commit

Permalink
Auto merge of #117094 - Nadrieril:warn-lint-on-arm, r=<try>
Browse files Browse the repository at this point in the history
Warn users who set `non_exhaustive_omitted_patterns` lint level on a match arm

Before #116734, the recommended usage of the [`non_exhaustive_omitted_patterns` lint](#89554) was:
```rust
match Bar::A {
    Bar::A => {},
    #[warn(non_exhaustive_omitted_patterns)]
    _ => {},
}
```
After #116734 this no longer makes sense, and recommended usage is now:
```rust
#[warn(non_exhaustive_omitted_patterns)]
match Bar::A {
    Bar::A => {},
    _ => {},
}
```

As you can guess, this silently breaks all uses of the lint that used the previous form. This is a problem in particular because `syn` recommends usage of this lint to its users in the old way. This PR emits a warning when the previous form is used so users can update.

r? `@cjgillot`
  • Loading branch information
bors committed Oct 27, 2023
2 parents 95f6a01 + d53f545 commit c6ca96d
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 24 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@ mir_build_non_exhaustive_omitted_pattern = some variants are not matched explici
.help = ensure that all variants are matched explicitly by adding the suggested match arms
.note = the matched value is of type `{$scrut_ty}` and the `non_exhaustive_omitted_patterns` attribute was found
mir_build_non_exhaustive_omitted_pattern_lint_on_arm = the `non_exhaustive_omitted_pattern` lint level must be set on the whole match
.help = it used to make sense to set the lint level on an individual match arm, but that is no longer the case
mir_build_non_exhaustive_patterns_type_not_empty = non-exhaustive patterns: type `{$ty}` is non-empty
.def_note = `{$peeled_ty}` defined here
.type_note = the matched value is of type `{$ty}`
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,11 @@ pub(crate) struct NonExhaustiveOmittedPattern<'tcx> {
pub uncovered: Uncovered<'tcx>,
}

#[derive(LintDiagnostic)]
#[diag(mir_build_non_exhaustive_omitted_pattern_lint_on_arm)]
#[help]
pub(crate) struct NonExhaustiveOmittedPatternLintOnArm;

#[derive(Subdiagnostic)]
#[label(mir_build_uncovered)]
pub(crate) struct Uncovered<'tcx> {
Expand Down
63 changes: 40 additions & 23 deletions compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@
use self::ArmType::*;
use self::Usefulness::*;
use super::deconstruct_pat::{Constructor, ConstructorSet, DeconstructedPat, WitnessPat};
use crate::errors::{NonExhaustiveOmittedPattern, Uncovered};
use crate::errors::{NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered};

use rustc_data_structures::captures::Captures;

Expand Down Expand Up @@ -1024,30 +1024,47 @@ pub(crate) fn compute_match_usefulness<'p, 'tcx>(

// Run the non_exhaustive_omitted_patterns lint. Only run on refutable patterns to avoid hitting
// `if let`s. Only run if the match is exhaustive otherwise the error is redundant.
if cx.refutable
&& non_exhaustiveness_witnesses.is_empty()
&& !matches!(
if cx.refutable && non_exhaustiveness_witnesses.is_empty() {
if !matches!(
cx.tcx.lint_level_at_node(NON_EXHAUSTIVE_OMITTED_PATTERNS, lint_root).0,
rustc_session::lint::Level::Allow
)
{
let pat_column = arms.iter().flat_map(|arm| arm.pat.flatten_or_pat()).collect::<Vec<_>>();
let witnesses = collect_nonexhaustive_missing_variants(cx, &pat_column);

if !witnesses.is_empty() {
// Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns`
// is not exhaustive enough.
//
// NB: The partner lint for structs lives in `compiler/rustc_hir_analysis/src/check/pat.rs`.
cx.tcx.emit_spanned_lint(
NON_EXHAUSTIVE_OMITTED_PATTERNS,
lint_root,
scrut_span,
NonExhaustiveOmittedPattern {
scrut_ty,
uncovered: Uncovered::new(scrut_span, cx, witnesses),
},
);
) {
let pat_column =
arms.iter().flat_map(|arm| arm.pat.flatten_or_pat()).collect::<Vec<_>>();
let witnesses = collect_nonexhaustive_missing_variants(cx, &pat_column);

if !witnesses.is_empty() {
// Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns`
// is not exhaustive enough.
//
// NB: The partner lint for structs lives in `compiler/rustc_hir_analysis/src/check/pat.rs`.
cx.tcx.emit_spanned_lint(
NON_EXHAUSTIVE_OMITTED_PATTERNS,
lint_root,
scrut_span,
NonExhaustiveOmittedPattern {
scrut_ty,
uncovered: Uncovered::new(scrut_span, cx, witnesses),
},
);
}
} else {
// We used to allow putting the `#[allow(non_exhaustive_omitted_patterns)]` on a match
// arm. This no longer makes sense so we warn users, to avoid silently breaking their
// usage of the lint.
for arm in arms {
if !matches!(
cx.tcx.lint_level_at_node(NON_EXHAUSTIVE_OMITTED_PATTERNS, arm.hir_id).0,
rustc_session::lint::Level::Allow
) {
cx.tcx.emit_spanned_lint(
NON_EXHAUSTIVE_OMITTED_PATTERNS,
arm.hir_id,
arm.pat.span(),
NonExhaustiveOmittedPatternLintOnArm,
);
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ fn repeat() -> ! {
}

pub fn f(x: Ordering) {
#[deny(non_exhaustive_omitted_patterns)]
match x {
Ordering::Relaxed => println!("relaxed"),
Ordering::Release => println!("release"),
Ordering::Acquire => println!("acquire"),
Ordering::AcqRel | Ordering::SeqCst => repeat(),
#[deny(non_exhaustive_omitted_patterns)]
_ => repeat(),
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
error: some variants are not matched explicitly
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:15:11
|
LL | match val {
| ^^^ pattern `NonExhaustiveEnum::Struct { .. }` not covered
|
= help: ensure that all variants are matched explicitly by adding the suggested match arms
= note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found
note: the lint level is defined here
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:14:12
|
LL | #[deny(non_exhaustive_omitted_patterns)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: some variants are not matched explicitly
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:23:11
|
LL | match val {
| ^^^ pattern `NonExhaustiveEnum::Struct { .. }` not covered
|
= help: ensure that all variants are matched explicitly by adding the suggested match arms
= note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found
note: the lint level is defined here
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:22:27
|
LL | #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: the `non_exhaustive_omitted_pattern` lint level must be set on the whole match
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:34:9
|
LL | _ => {}
| ^
|
= help: it used to make sense to set the lint level on an individual match arm, but that is no longer the case
note: the lint level is defined here
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:33:16
|
LL | #[deny(non_exhaustive_omitted_patterns)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: the `non_exhaustive_omitted_pattern` lint level must be set on the whole match
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:41:9
|
LL | _ => {}
| ^
|
= help: it used to make sense to set the lint level on an individual match arm, but that is no longer the case
note: the lint level is defined here
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:40:31
|
LL | #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: the `non_exhaustive_omitted_pattern` lint level must be set on the whole match
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:48:9
|
LL | _ => {}
| ^
|
= help: it used to make sense to set the lint level on an individual match arm, but that is no longer the case
note: the lint level is defined here
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:47:31
|
LL | #[cfg_attr(lint, warn(non_exhaustive_omitted_patterns))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 4 previous errors; 1 warning emitted

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
error: some variants are not matched explicitly
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:15:11
|
LL | match val {
| ^^^ pattern `NonExhaustiveEnum::Struct { .. }` not covered
|
= help: ensure that all variants are matched explicitly by adding the suggested match arms
= note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found
note: the lint level is defined here
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:14:12
|
LL | #[deny(non_exhaustive_omitted_patterns)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: the `non_exhaustive_omitted_pattern` lint level must be set on the whole match
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:34:9
|
LL | _ => {}
| ^
|
= help: it used to make sense to set the lint level on an individual match arm, but that is no longer the case
note: the lint level is defined here
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:33:16
|
LL | #[deny(non_exhaustive_omitted_patterns)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// revisions: normal lint
// Test that putting the lint level on a match arm emits a warning, as this was previously
// meaningful and is no longer.
#![feature(non_exhaustive_omitted_patterns_lint)]

// aux-build:enums.rs
extern crate enums;

use enums::NonExhaustiveEnum;

fn main() {
let val = NonExhaustiveEnum::Unit;

#[deny(non_exhaustive_omitted_patterns)]
match val {
//~^ ERROR some variants are not matched explicitly
NonExhaustiveEnum::Unit => {}
NonExhaustiveEnum::Tuple(_) => {}
_ => {}
}

#[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
match val {
//[lint]~^ ERROR some variants are not matched explicitly
NonExhaustiveEnum::Unit => {}
NonExhaustiveEnum::Tuple(_) => {}
_ => {}
}

match val {
NonExhaustiveEnum::Unit => {}
NonExhaustiveEnum::Tuple(_) => {}
#[deny(non_exhaustive_omitted_patterns)]
_ => {} //~ ERROR lint level must be set on the whole match
}

match val {
NonExhaustiveEnum::Unit => {}
NonExhaustiveEnum::Tuple(_) => {}
#[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
_ => {} //[lint]~ ERROR lint level must be set on the whole match
}

match val {
NonExhaustiveEnum::Unit => {}
NonExhaustiveEnum::Tuple(_) => {}
#[cfg_attr(lint, warn(non_exhaustive_omitted_patterns))]
_ => {} //[lint]~ WARN lint level must be set on the whole match
}
}

0 comments on commit c6ca96d

Please sign in to comment.