diff options
| author | Michael Goulet <michael@errs.io> | 2023-05-25 13:58:00 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2023-05-25 13:58:00 -0700 |
| commit | 9d4527bc8099e0e22df2b32d055aaefa6a6e759f (patch) | |
| tree | b06464234e75b488e44deb38f9e3c708d96de5a8 | |
| parent | bd7e8b5ef9617d3ac7fbd79596ee1754beef3206 (diff) | |
| parent | 3a0358783695991a9a06c9c8f96906087b95e400 (diff) | |
| download | rust-9d4527bc8099e0e22df2b32d055aaefa6a6e759f.tar.gz rust-9d4527bc8099e0e22df2b32d055aaefa6a6e759f.zip | |
Rollup merge of #111757 - lowr:fix/lint-attr-on-match-arm, r=eholk
Consider lint check attributes on match arms
Currently, lint check attributes on match arms have no effect for some lints. This PR makes some lint passes to take those attributes into account.
- `LateContextAndPass` for late lint doesn't update `last_node_with_lint_attrs` when it visits match arms. This leads to lint check attributes on match arms taking no effects on late lints that operate on the arms' pattern:
```rust
match value {
#[deny(non_snake_case)]
PAT => {} // `non_snake_case` only warned due to default lint level
}
```
To be honest, I'm not sure whether this is intentional or just an oversight. I've dug the implementation history and searched up issues/PRs but couldn't find any discussion on this.
- `MatchVisitor` doesn't update its lint level when it visits match arms. This leads to check lint attributes on match arms taking no effect on some lints handled by this visitor, namely: `bindings_with_variant_name` and `irrefutable_let_patterns`.
This seems to be a fallout from #108504. Before 05082f57afbf5d2e8fd7fb67719336d78b58e759, when the visitor operated on HIR rather than THIR, check lint attributes for the said lints were effective. [This playground][play] compiles successfully on current stable (1.69) but fails on current beta and nightly.
I wasn't sure where best to place the test for this. Let me know if there's a better place.
[play]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=38432b79e535cb175f8f7d6d236d29c3
[play-match]: https://play.rust-lang.org/?version=beta&mode=debug&edition=2021&gist=629aa71b7c84b269beadeba664e2221d
| -rw-r--r-- | compiler/rustc_lint/src/late.rs | 6 | ||||
| -rw-r--r-- | compiler/rustc_mir_build/src/thir/pattern/check_match.rs | 58 | ||||
| -rw-r--r-- | tests/ui/lint/lint-attr-everywhere-early.rs | 8 | ||||
| -rw-r--r-- | tests/ui/lint/lint-attr-everywhere-early.stderr | 48 | ||||
| -rw-r--r-- | tests/ui/lint/lint-attr-everywhere-late.rs | 5 | ||||
| -rw-r--r-- | tests/ui/lint/lint-attr-everywhere-late.stderr | 56 | ||||
| -rw-r--r-- | tests/ui/lint/lint-match-arms-2.rs | 24 | ||||
| -rw-r--r-- | tests/ui/lint/lint-match-arms-2.stderr | 29 |
8 files changed, 170 insertions, 64 deletions
diff --git a/compiler/rustc_lint/src/late.rs b/compiler/rustc_lint/src/late.rs index c9781a72704..8a4a451f8a8 100644 --- a/compiler/rustc_lint/src/late.rs +++ b/compiler/rustc_lint/src/late.rs @@ -240,8 +240,10 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas } fn visit_arm(&mut self, a: &'tcx hir::Arm<'tcx>) { - lint_callback!(self, check_arm, a); - hir_visit::walk_arm(self, a); + self.with_lint_attrs(a.hir_id, |cx| { + lint_callback!(cx, check_arm, a); + hir_visit::walk_arm(cx, a); + }) } fn visit_generic_param(&mut self, p: &'tcx hir::GenericParam<'tcx>) { diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index 5559e8b3940..c11a4f7229d 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -90,35 +90,34 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for MatchVisitor<'a, '_, 'tcx> { #[instrument(level = "trace", skip(self))] fn visit_arm(&mut self, arm: &Arm<'tcx>) { - match arm.guard { - Some(Guard::If(expr)) => { - self.with_let_source(LetSource::IfLetGuard, |this| { - this.visit_expr(&this.thir[expr]) - }); - } - Some(Guard::IfLet(ref pat, expr)) => { - self.with_let_source(LetSource::IfLetGuard, |this| { - this.check_let(pat, expr, LetSource::IfLetGuard, pat.span); - this.visit_pat(pat); - this.visit_expr(&this.thir[expr]); - }); + self.with_lint_level(arm.lint_level, |this| { + match arm.guard { + Some(Guard::If(expr)) => { + this.with_let_source(LetSource::IfLetGuard, |this| { + this.visit_expr(&this.thir[expr]) + }); + } + Some(Guard::IfLet(ref pat, expr)) => { + this.with_let_source(LetSource::IfLetGuard, |this| { + this.check_let(pat, expr, LetSource::IfLetGuard, pat.span); + this.visit_pat(pat); + this.visit_expr(&this.thir[expr]); + }); + } + None => {} } - None => {} - } - self.visit_pat(&arm.pattern); - self.visit_expr(&self.thir[arm.body]); + this.visit_pat(&arm.pattern); + this.visit_expr(&self.thir[arm.body]); + }); } #[instrument(level = "trace", skip(self))] fn visit_expr(&mut self, ex: &Expr<'tcx>) { match ex.kind { ExprKind::Scope { value, lint_level, .. } => { - let old_lint_level = self.lint_level; - if let LintLevel::Explicit(hir_id) = lint_level { - self.lint_level = hir_id; - } - self.visit_expr(&self.thir[value]); - self.lint_level = old_lint_level; + self.with_lint_level(lint_level, |this| { + this.visit_expr(&this.thir[value]); + }); return; } ExprKind::If { cond, then, else_opt, if_then_scope: _ } => { @@ -190,6 +189,17 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> { self.let_source = old_let_source; } + fn with_lint_level(&mut self, new_lint_level: LintLevel, f: impl FnOnce(&mut Self)) { + if let LintLevel::Explicit(hir_id) = new_lint_level { + let old_lint_level = self.lint_level; + self.lint_level = hir_id; + f(self); + self.lint_level = old_lint_level; + } else { + f(self); + } + } + fn check_patterns(&self, pat: &Pat<'tcx>, rf: RefutableFlag) { pat.walk_always(|pat| check_borrow_conflicts_in_at_patterns(self, pat)); check_for_bindings_named_same_as_variants(self, pat, rf); @@ -236,7 +246,9 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> { for &arm in arms { // Check the arm for some things unrelated to exhaustiveness. let arm = &self.thir.arms[arm]; - self.check_patterns(&arm.pattern, Refutable); + self.with_lint_level(arm.lint_level, |this| { + this.check_patterns(&arm.pattern, Refutable); + }); } let tarms: Vec<_> = arms diff --git a/tests/ui/lint/lint-attr-everywhere-early.rs b/tests/ui/lint/lint-attr-everywhere-early.rs index fd0c4b43e05..0c820ef9a29 100644 --- a/tests/ui/lint/lint-attr-everywhere-early.rs +++ b/tests/ui/lint/lint-attr-everywhere-early.rs @@ -134,6 +134,14 @@ fn expressions() { } } + match f { + #[deny(ellipsis_inclusive_range_patterns)] + Match{f1: 0...100} => {} + //~^ ERROR range patterns are deprecated + //~| WARNING this is accepted in the current edition + _ => {} + } + // Statement Block { #![deny(unsafe_code)] diff --git a/tests/ui/lint/lint-attr-everywhere-early.stderr b/tests/ui/lint/lint-attr-everywhere-early.stderr index d6c6d5faef2..fac0eb4faff 100644 --- a/tests/ui/lint/lint-attr-everywhere-early.stderr +++ b/tests/ui/lint/lint-attr-everywhere-early.stderr @@ -384,92 +384,106 @@ note: the lint level is defined here LL | #[deny(while_true)] | ^^^^^^^^^^ +error: `...` range patterns are deprecated + --> $DIR/lint-attr-everywhere-early.rs:139:20 + | +LL | Match{f1: 0...100} => {} + | ^^^ help: use `..=` for an inclusive range + | + = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021! + = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html> +note: the lint level is defined here + --> $DIR/lint-attr-everywhere-early.rs:138:16 + | +LL | #[deny(ellipsis_inclusive_range_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + error: usage of an `unsafe` block - --> $DIR/lint-attr-everywhere-early.rs:140:9 + --> $DIR/lint-attr-everywhere-early.rs:148:9 | LL | unsafe {} | ^^^^^^^^^ | note: the lint level is defined here - --> $DIR/lint-attr-everywhere-early.rs:139:17 + --> $DIR/lint-attr-everywhere-early.rs:147:17 | LL | #![deny(unsafe_code)] | ^^^^^^^^^^^ error: usage of an `unsafe` block - --> $DIR/lint-attr-everywhere-early.rs:144:9 + --> $DIR/lint-attr-everywhere-early.rs:152:9 | LL | unsafe {} | ^^^^^^^^^ | note: the lint level is defined here - --> $DIR/lint-attr-everywhere-early.rs:143:16 + --> $DIR/lint-attr-everywhere-early.rs:151:16 | LL | #[deny(unsafe_code)] | ^^^^^^^^^^^ error: usage of an `unsafe` block - --> $DIR/lint-attr-everywhere-early.rs:149:5 + --> $DIR/lint-attr-everywhere-early.rs:157:5 | LL | unsafe {}; | ^^^^^^^^^ | note: the lint level is defined here - --> $DIR/lint-attr-everywhere-early.rs:148:12 + --> $DIR/lint-attr-everywhere-early.rs:156:12 | LL | #[deny(unsafe_code)] | ^^^^^^^^^^^ error: usage of an `unsafe` block - --> $DIR/lint-attr-everywhere-early.rs:151:27 + --> $DIR/lint-attr-everywhere-early.rs:159:27 | LL | [#[deny(unsafe_code)] unsafe {123}]; | ^^^^^^^^^^^^ | note: the lint level is defined here - --> $DIR/lint-attr-everywhere-early.rs:151:13 + --> $DIR/lint-attr-everywhere-early.rs:159:13 | LL | [#[deny(unsafe_code)] unsafe {123}]; | ^^^^^^^^^^^ error: usage of an `unsafe` block - --> $DIR/lint-attr-everywhere-early.rs:152:27 + --> $DIR/lint-attr-everywhere-early.rs:160:27 | LL | (#[deny(unsafe_code)] unsafe {123},); | ^^^^^^^^^^^^ | note: the lint level is defined here - --> $DIR/lint-attr-everywhere-early.rs:152:13 + --> $DIR/lint-attr-everywhere-early.rs:160:13 | LL | (#[deny(unsafe_code)] unsafe {123},); | ^^^^^^^^^^^ error: usage of an `unsafe` block - --> $DIR/lint-attr-everywhere-early.rs:154:31 + --> $DIR/lint-attr-everywhere-early.rs:162:31 | LL | call(#[deny(unsafe_code)] unsafe {123}); | ^^^^^^^^^^^^ | note: the lint level is defined here - --> $DIR/lint-attr-everywhere-early.rs:154:17 + --> $DIR/lint-attr-everywhere-early.rs:162:17 | LL | call(#[deny(unsafe_code)] unsafe {123}); | ^^^^^^^^^^^ error: usage of an `unsafe` block - --> $DIR/lint-attr-everywhere-early.rs:156:38 + --> $DIR/lint-attr-everywhere-early.rs:164:38 | LL | TupleStruct(#[deny(unsafe_code)] unsafe {123}); | ^^^^^^^^^^^^ | note: the lint level is defined here - --> $DIR/lint-attr-everywhere-early.rs:156:24 + --> $DIR/lint-attr-everywhere-early.rs:164:24 | LL | TupleStruct(#[deny(unsafe_code)] unsafe {123}); | ^^^^^^^^^^^ error: `...` range patterns are deprecated - --> $DIR/lint-attr-everywhere-early.rs:167:18 + --> $DIR/lint-attr-everywhere-early.rs:175:18 | LL | f1: 0...100, | ^^^ help: use `..=` for an inclusive range @@ -477,10 +491,10 @@ LL | f1: 0...100, = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021! = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html> note: the lint level is defined here - --> $DIR/lint-attr-everywhere-early.rs:166:20 + --> $DIR/lint-attr-everywhere-early.rs:174:20 | LL | #[deny(ellipsis_inclusive_range_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 36 previous errors +error: aborting due to 37 previous errors diff --git a/tests/ui/lint/lint-attr-everywhere-late.rs b/tests/ui/lint/lint-attr-everywhere-late.rs index 1055157d602..a24355babb6 100644 --- a/tests/ui/lint/lint-attr-everywhere-late.rs +++ b/tests/ui/lint/lint-attr-everywhere-late.rs @@ -162,6 +162,11 @@ fn expressions() { } } + match 123 { + #[deny(non_snake_case)] + ARM_VAR => {} //~ ERROR variable `ARM_VAR` should have a snake case name + } + // Statement Block { #![deny(enum_intrinsics_non_enums)] diff --git a/tests/ui/lint/lint-attr-everywhere-late.stderr b/tests/ui/lint/lint-attr-everywhere-late.stderr index a69c2e0ef2b..9587556b0c1 100644 --- a/tests/ui/lint/lint-attr-everywhere-late.stderr +++ b/tests/ui/lint/lint-attr-everywhere-late.stderr @@ -305,124 +305,136 @@ note: the lint level is defined here LL | #[deny(enum_intrinsics_non_enums)] | ^^^^^^^^^^^^^^^^^^^^^^^^^ +error: variable `ARM_VAR` should have a snake case name + --> $DIR/lint-attr-everywhere-late.rs:167:9 + | +LL | ARM_VAR => {} + | ^^^^^^^ help: convert the identifier to snake case: `arm_var` + | +note: the lint level is defined here + --> $DIR/lint-attr-everywhere-late.rs:166:16 + | +LL | #[deny(non_snake_case)] + | ^^^^^^^^^^^^^^ + error: the return value of `mem::discriminant` is unspecified when called with a non-enum type - --> $DIR/lint-attr-everywhere-late.rs:168:9 + --> $DIR/lint-attr-everywhere-late.rs:173:9 | LL | discriminant::<i32>(&123); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `i32`, which is not an enum. - --> $DIR/lint-attr-everywhere-late.rs:168:29 + --> $DIR/lint-attr-everywhere-late.rs:173:29 | LL | discriminant::<i32>(&123); | ^^^^ note: the lint level is defined here - --> $DIR/lint-attr-everywhere-late.rs:167:17 + --> $DIR/lint-attr-everywhere-late.rs:172:17 | LL | #![deny(enum_intrinsics_non_enums)] | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type - --> $DIR/lint-attr-everywhere-late.rs:172:9 + --> $DIR/lint-attr-everywhere-late.rs:177:9 | LL | discriminant::<i32>(&123); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `i32`, which is not an enum. - --> $DIR/lint-attr-everywhere-late.rs:172:29 + --> $DIR/lint-attr-everywhere-late.rs:177:29 | LL | discriminant::<i32>(&123); | ^^^^ note: the lint level is defined here - --> $DIR/lint-attr-everywhere-late.rs:171:16 + --> $DIR/lint-attr-everywhere-late.rs:176:16 | LL | #[deny(enum_intrinsics_non_enums)] | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type - --> $DIR/lint-attr-everywhere-late.rs:177:5 + --> $DIR/lint-attr-everywhere-late.rs:182:5 | LL | discriminant::<i32>(&123); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `i32`, which is not an enum. - --> $DIR/lint-attr-everywhere-late.rs:177:25 + --> $DIR/lint-attr-everywhere-late.rs:182:25 | LL | discriminant::<i32>(&123); | ^^^^ note: the lint level is defined here - --> $DIR/lint-attr-everywhere-late.rs:176:12 + --> $DIR/lint-attr-everywhere-late.rs:181:12 | LL | #[deny(enum_intrinsics_non_enums)] | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type - --> $DIR/lint-attr-everywhere-late.rs:179:41 + --> $DIR/lint-attr-everywhere-late.rs:184:41 | LL | [#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123)]; | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `i32`, which is not an enum. - --> $DIR/lint-attr-everywhere-late.rs:179:61 + --> $DIR/lint-attr-everywhere-late.rs:184:61 | LL | [#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123)]; | ^^^^ note: the lint level is defined here - --> $DIR/lint-attr-everywhere-late.rs:179:13 + --> $DIR/lint-attr-everywhere-late.rs:184:13 | LL | [#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123)]; | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type - --> $DIR/lint-attr-everywhere-late.rs:180:41 + --> $DIR/lint-attr-everywhere-late.rs:185:41 | LL | (#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123),); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `i32`, which is not an enum. - --> $DIR/lint-attr-everywhere-late.rs:180:61 + --> $DIR/lint-attr-everywhere-late.rs:185:61 | LL | (#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123),); | ^^^^ note: the lint level is defined here - --> $DIR/lint-attr-everywhere-late.rs:180:13 + --> $DIR/lint-attr-everywhere-late.rs:185:13 | LL | (#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123),); | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type - --> $DIR/lint-attr-everywhere-late.rs:182:45 + --> $DIR/lint-attr-everywhere-late.rs:187:45 | LL | call(#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123)); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `i32`, which is not an enum. - --> $DIR/lint-attr-everywhere-late.rs:182:65 + --> $DIR/lint-attr-everywhere-late.rs:187:65 | LL | call(#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123)); | ^^^^ note: the lint level is defined here - --> $DIR/lint-attr-everywhere-late.rs:182:17 + --> $DIR/lint-attr-everywhere-late.rs:187:17 | LL | call(#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123)); | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type - --> $DIR/lint-attr-everywhere-late.rs:184:52 + --> $DIR/lint-attr-everywhere-late.rs:189:52 | LL | TupleStruct(#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123)); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `i32`, which is not an enum. - --> $DIR/lint-attr-everywhere-late.rs:184:72 + --> $DIR/lint-attr-everywhere-late.rs:189:72 | LL | TupleStruct(#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123)); | ^^^^ note: the lint level is defined here - --> $DIR/lint-attr-everywhere-late.rs:184:24 + --> $DIR/lint-attr-everywhere-late.rs:189:24 | LL | TupleStruct(#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123)); | ^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 31 previous errors +error: aborting due to 32 previous errors diff --git a/tests/ui/lint/lint-match-arms-2.rs b/tests/ui/lint/lint-match-arms-2.rs new file mode 100644 index 00000000000..0c1146339c4 --- /dev/null +++ b/tests/ui/lint/lint-match-arms-2.rs @@ -0,0 +1,24 @@ +#![feature(if_let_guard)] +#![allow(unused, non_snake_case)] + +enum E { + A, +} + +#[allow(bindings_with_variant_name, irrefutable_let_patterns)] +fn foo() { + match E::A { + #[deny(bindings_with_variant_name)] + A => {} + //~^ ERROR pattern binding `A` is named the same as one of the variants of the type `E` + } + + match &E::A { + #[deny(irrefutable_let_patterns)] + a if let b = a => {} + //~^ ERROR irrefutable `if let` guard pattern + _ => {} + } +} + +fn main() { } diff --git a/tests/ui/lint/lint-match-arms-2.stderr b/tests/ui/lint/lint-match-arms-2.stderr new file mode 100644 index 00000000000..062d5c12e96 --- /dev/null +++ b/tests/ui/lint/lint-match-arms-2.stderr @@ -0,0 +1,29 @@ +error[E0170]: pattern binding `A` is named the same as one of the variants of the type `E` + --> $DIR/lint-match-arms-2.rs:12:9 + | +LL | A => {} + | ^ help: to match on the variant, qualify the path: `E::A` + | +note: the lint level is defined here + --> $DIR/lint-match-arms-2.rs:11:16 + | +LL | #[deny(bindings_with_variant_name)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: irrefutable `if let` guard pattern + --> $DIR/lint-match-arms-2.rs:18:18 + | +LL | a if let b = a => {} + | ^ + | + = note: this pattern will always match, so the guard is useless + = help: consider removing the guard and adding a `let` inside the match arm +note: the lint level is defined here + --> $DIR/lint-match-arms-2.rs:17:16 + | +LL | #[deny(irrefutable_let_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0170`. |
