From 5cf692973f1c14f1bfb25f05aada42f1bcda6e45 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 23 Oct 2023 18:27:33 +0200 Subject: [PATCH 1/2] Add tests --- ...tted-patterns-dont-lint-on-arm.lint.stderr | 30 +++++++++++ ...ed-patterns-dont-lint-on-arm.normal.stderr | 16 ++++++ .../omitted-patterns-dont-lint-on-arm.rs | 50 +++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr create mode 100644 tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr create mode 100644 tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr new file mode 100644 index 0000000000000..c58b07d1aef2f --- /dev/null +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr @@ -0,0 +1,30 @@ +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: aborting due to 2 previous errors + diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr new file mode 100644 index 0000000000000..2d94a09de6344 --- /dev/null +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr @@ -0,0 +1,16 @@ +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: aborting due to previous error + diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs new file mode 100644 index 0000000000000..9411d8f085bbf --- /dev/null +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs @@ -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)] + _ => {} + } + + match val { + NonExhaustiveEnum::Unit => {} + NonExhaustiveEnum::Tuple(_) => {} + #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))] + _ => {} + } + + match val { + NonExhaustiveEnum::Unit => {} + NonExhaustiveEnum::Tuple(_) => {} + #[cfg_attr(lint, warn(non_exhaustive_omitted_patterns))] + _ => {} + } +} From d53f545f76f9207033a03d93e3acd12f0a0a7d69 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 23 Oct 2023 18:55:04 +0200 Subject: [PATCH 2/2] Warn when lint level is set on a match arm --- compiler/rustc_mir_build/messages.ftl | 3 + compiler/rustc_mir_build/src/errors.rs | 5 ++ .../src/thir/pattern/usefulness.rs | 63 ++++++++++++------- .../ui/match_same_arms_non_exhaustive.rs | 2 +- ...tted-patterns-dont-lint-on-arm.lint.stderr | 41 +++++++++++- ...ed-patterns-dont-lint-on-arm.normal.stderr | 15 ++++- .../omitted-patterns-dont-lint-on-arm.rs | 6 +- 7 files changed, 106 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index db3886b32beb4..c8b8771bcd17f 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -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}` diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index c09dd1864185c..4320a1920055d 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -794,6 +794,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> { diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs index a8cee5a61edb5..f34634be548eb 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs @@ -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; @@ -1021,30 +1021,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::>(); - 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::>(); + 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, + ); + } + } } } diff --git a/src/tools/clippy/tests/ui/match_same_arms_non_exhaustive.rs b/src/tools/clippy/tests/ui/match_same_arms_non_exhaustive.rs index e1b95aa577604..5c277f925a8f1 100644 --- a/src/tools/clippy/tests/ui/match_same_arms_non_exhaustive.rs +++ b/src/tools/clippy/tests/ui/match_same_arms_non_exhaustive.rs @@ -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(), } } diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr index c58b07d1aef2f..b0629c6de1a48 100644 --- a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr @@ -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 diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr index 2d94a09de6344..a7a2ad3a5f0d3 100644 --- a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr @@ -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 diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs index 9411d8f085bbf..ac7b7cd93fd1f 100644 --- a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs @@ -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 } }