Skip to content

Commit

Permalink
Warn when lint level is set on a match arm
Browse files Browse the repository at this point in the history
  • Loading branch information
Nadrieril committed Oct 27, 2023
1 parent c5aab79 commit 485b566
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 27 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
62 changes: 41 additions & 21 deletions compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,10 @@ use self::Usefulness::*;
use super::deconstruct_pat::{
Constructor, ConstructorSet, DeconstructedPat, IntRange, SplitConstructorSet, WitnessPat,
};
use crate::errors::{NonExhaustiveOmittedPattern, Overlap, OverlappingRangeEndpoints, Uncovered};
use crate::errors::{
NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Overlap,
OverlappingRangeEndpoints, Uncovered,
};

use rustc_data_structures::captures::Captures;

Expand Down Expand Up @@ -1148,28 +1151,45 @@ 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 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 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
Expand Up @@ -26,5 +26,44 @@ note: the lint level is defined here
LL | #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors
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
Expand Up @@ -12,5 +12,18 @@ note: the lint level is defined here
LL | #[deny(non_exhaustive_omitted_patterns)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error
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
Expand Up @@ -31,20 +31,20 @@ fn main() {
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 485b566

Please sign in to comment.