Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn users who set non_exhaustive_omitted_patterns lint level on a match arm #117094

Merged
merged 3 commits into from
Nov 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ 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 lint level must be set on the whole match
.help = it no longer has any effect to set the lint level on an individual match arm
.label = remove this attribute
.suggestion = set the lint level on the whole match

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
12 changes: 12 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,18 @@ 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 {
#[label]
pub lint_span: Span,
#[suggestion(code = "#[{lint_level}({lint_name})]\n", applicability = "maybe-incorrect")]
pub suggest_lint_on_match: Option<Span>,
pub lint_level: &'static str,
pub lint_name: &'static str,
}

#[derive(Subdiagnostic)]
#[label(mir_build_uncovered)]
pub(crate) struct Uncovered<'tcx> {
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,11 @@ impl<'thir, 'p, 'tcx> MatchVisitor<'thir, 'p, 'tcx> {
}
}

fn new_cx(&self, refutability: RefutableFlag) -> MatchCheckCtxt<'p, 'tcx> {
fn new_cx(
&self,
refutability: RefutableFlag,
match_span: Option<Span>,
) -> MatchCheckCtxt<'p, 'tcx> {
let refutable = match refutability {
Irrefutable => false,
Refutable => true,
Expand All @@ -295,6 +299,7 @@ impl<'thir, 'p, 'tcx> MatchVisitor<'thir, 'p, 'tcx> {
param_env: self.param_env,
module: self.tcx.parent_module(self.lint_level).to_def_id(),
pattern_arena: &self.pattern_arena,
match_span,
refutable,
}
}
Expand Down Expand Up @@ -325,7 +330,7 @@ impl<'thir, 'p, 'tcx> MatchVisitor<'thir, 'p, 'tcx> {
source: hir::MatchSource,
expr_span: Span,
) {
let cx = self.new_cx(Refutable);
let cx = self.new_cx(Refutable, Some(expr_span));

let mut tarms = Vec::with_capacity(arms.len());
for &arm in arms {
Expand Down Expand Up @@ -448,7 +453,7 @@ impl<'thir, 'p, 'tcx> MatchVisitor<'thir, 'p, 'tcx> {
pat: &Pat<'tcx>,
refutability: RefutableFlag,
) -> Result<(MatchCheckCtxt<'p, 'tcx>, UsefulnessReport<'p, 'tcx>), ErrorGuaranteed> {
let cx = self.new_cx(refutability);
let cx = self.new_cx(refutability, None);
let pat = self.lower_pattern(&cx, pat)?;
let arms = [MatchArm { pat, hir_id: self.lint_level, has_guard: false }];
let report = compute_match_usefulness(&cx, &arms, self.lint_level, pat.ty(), pat.span());
Expand Down
69 changes: 48 additions & 21 deletions compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,10 @@ use super::deconstruct_pat::{
Constructor, ConstructorSet, DeconstructedPat, IntRange, MaybeInfiniteInt, SplitConstructorSet,
WitnessPat,
};
use crate::errors::{NonExhaustiveOmittedPattern, Overlap, OverlappingRangeEndpoints, Uncovered};
use crate::errors::{
NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Overlap,
OverlappingRangeEndpoints, Uncovered,
};

use rustc_data_structures::captures::Captures;

Expand All @@ -337,6 +340,8 @@ pub(crate) struct MatchCheckCtxt<'p, 'tcx> {
pub(crate) module: DefId,
pub(crate) param_env: ty::ParamEnv<'tcx>,
pub(crate) pattern_arena: &'p TypedArena<DeconstructedPat<'p, 'tcx>>,
/// The span of the whole match, if applicable.
pub(crate) match_span: Option<Span>,
/// Only produce `NON_EXHAUSTIVE_OMITTED_PATTERNS` lint on refutable patterns.
pub(crate) refutable: bool,
}
Expand Down Expand Up @@ -1149,28 +1154,50 @@ 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 {
let (lint_level, lint_level_source) =
cx.tcx.lint_level_at_node(NON_EXHAUSTIVE_OMITTED_PATTERNS, arm.hir_id);
if !matches!(lint_level, rustc_session::lint::Level::Allow) {
let decorator = NonExhaustiveOmittedPatternLintOnArm {
lint_span: lint_level_source.span(),
suggest_lint_on_match: cx.match_span.map(|span| span.shrink_to_lo()),
lint_level: lint_level.as_str(),
lint_name: "non_exhaustive_omitted_patterns",
};

use rustc_errors::DecorateLint;
let mut err = cx.tcx.sess.struct_span_warn(arm.pat.span(), "");
err.set_primary_message(decorator.msg());
decorator.decorate_lint(&mut err);
err.emit();
}
}
}
}

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,75 @@
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))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: the lint level must be set on the whole match
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:34:9
|
LL | #[deny(non_exhaustive_omitted_patterns)]
| ------------------------------- remove this attribute
LL | _ => {}
| ^
|
= help: it no longer has any effect to set the lint level on an individual match arm
help: set the lint level on the whole match
|
LL + #[deny(non_exhaustive_omitted_patterns)]
LL | match val {
|

warning: the lint level must be set on the whole match
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:42:9
|
LL | #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
| ------------------------------- remove this attribute
LL | _ => {}
| ^
|
= help: it no longer has any effect to set the lint level on an individual match arm
help: set the lint level on the whole match
|
LL + #[deny(non_exhaustive_omitted_patterns)]
LL | match val {
|

warning: the lint level must be set on the whole match
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:50:9
|
LL | #[cfg_attr(lint, warn(non_exhaustive_omitted_patterns))]
| ------------------------------- remove this attribute
LL | _ => {}
| ^
|
= help: it no longer has any effect to set the lint level on an individual match arm
help: set the lint level on the whole match
|
LL + #[warn(non_exhaustive_omitted_patterns)]
LL | match val {
|

error: aborting due to 2 previous errors; 3 warnings emitted

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
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)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: the lint level must be set on the whole match
--> $DIR/omitted-patterns-dont-lint-on-arm.rs:34:9
|
LL | #[deny(non_exhaustive_omitted_patterns)]
| ------------------------------- remove this attribute
LL | _ => {}
| ^
|
= help: it no longer has any effect to set the lint level on an individual match arm
help: set the lint level on the whole match
|
LL + #[deny(non_exhaustive_omitted_patterns)]
LL | match val {
|

error: aborting due to previous error; 1 warning emitted

Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// 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)]
_ => {}
}
//~^^ WARN lint level must be set on the whole match

match val {
NonExhaustiveEnum::Unit => {}
NonExhaustiveEnum::Tuple(_) => {}
#[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
_ => {}
}
//[lint]~^^ WARN 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
}
Loading