Skip to content

Commit 9d4527b

Browse files
compiler-errorscompiler-errors
authored
Rollup merge of rust-lang#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 rust-lang#108504. Before 05082f5, 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
2 parents bd7e8b5 + 3a03587 commit 9d4527b

File tree

8 files changed

+170
-64
lines changed

8 files changed

+170
-64
lines changed

‎compiler/rustc_lint/src/late.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,10 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas
240240
}
241241

242242
fn visit_arm(&mut self, a: &'tcx hir::Arm<'tcx>) {
243-
lint_callback!(self, check_arm, a);
244-
hir_visit::walk_arm(self, a);
243+
self.with_lint_attrs(a.hir_id, |cx| {
244+
lint_callback!(cx, check_arm, a);
245+
hir_visit::walk_arm(cx, a);
246+
})
245247
}
246248

247249
fn visit_generic_param(&mut self, p: &'tcx hir::GenericParam<'tcx>) {

‎compiler/rustc_mir_build/src/thir/pattern/check_match.rs

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -90,35 +90,34 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for MatchVisitor<'a, '_, 'tcx> {
9090

9191
#[instrument(level = "trace", skip(self))]
9292
fn visit_arm(&mut self, arm: &Arm<'tcx>) {
93-
match arm.guard {
94-
Some(Guard::If(expr)) => {
95-
self.with_let_source(LetSource::IfLetGuard, |this| {
96-
this.visit_expr(&this.thir[expr])
97-
});
98-
}
99-
Some(Guard::IfLet(ref pat, expr)) => {
100-
self.with_let_source(LetSource::IfLetGuard, |this| {
101-
this.check_let(pat, expr, LetSource::IfLetGuard, pat.span);
102-
this.visit_pat(pat);
103-
this.visit_expr(&this.thir[expr]);
104-
});
93+
self.with_lint_level(arm.lint_level, |this| {
94+
match arm.guard {
95+
Some(Guard::If(expr)) => {
96+
this.with_let_source(LetSource::IfLetGuard, |this| {
97+
this.visit_expr(&this.thir[expr])
98+
});
99+
}
100+
Some(Guard::IfLet(ref pat, expr)) => {
101+
this.with_let_source(LetSource::IfLetGuard, |this| {
102+
this.check_let(pat, expr, LetSource::IfLetGuard, pat.span);
103+
this.visit_pat(pat);
104+
this.visit_expr(&this.thir[expr]);
105+
});
106+
}
107+
None => {}
105108
}
106-
None => {}
107-
}
108-
self.visit_pat(&arm.pattern);
109-
self.visit_expr(&self.thir[arm.body]);
109+
this.visit_pat(&arm.pattern);
110+
this.visit_expr(&self.thir[arm.body]);
111+
});
110112
}
111113

112114
#[instrument(level = "trace", skip(self))]
113115
fn visit_expr(&mut self, ex: &Expr<'tcx>) {
114116
match ex.kind {
115117
ExprKind::Scope { value, lint_level, .. } => {
116-
let old_lint_level = self.lint_level;
117-
if let LintLevel::Explicit(hir_id) = lint_level {
118-
self.lint_level = hir_id;
119-
}
120-
self.visit_expr(&self.thir[value]);
121-
self.lint_level = old_lint_level;
118+
self.with_lint_level(lint_level, |this| {
119+
this.visit_expr(&this.thir[value]);
120+
});
122121
return;
123122
}
124123
ExprKind::If { cond, then, else_opt, if_then_scope: _ } => {
@@ -190,6 +189,17 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
190189
self.let_source = old_let_source;
191190
}
192191

192+
fn with_lint_level(&mut self, new_lint_level: LintLevel, f: impl FnOnce(&mut Self)) {
193+
if let LintLevel::Explicit(hir_id) = new_lint_level {
194+
let old_lint_level = self.lint_level;
195+
self.lint_level = hir_id;
196+
f(self);
197+
self.lint_level = old_lint_level;
198+
} else {
199+
f(self);
200+
}
201+
}
202+
193203
fn check_patterns(&self, pat: &Pat<'tcx>, rf: RefutableFlag) {
194204
pat.walk_always(|pat| check_borrow_conflicts_in_at_patterns(self, pat));
195205
check_for_bindings_named_same_as_variants(self, pat, rf);
@@ -236,7 +246,9 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
236246
for &arm in arms {
237247
// Check the arm for some things unrelated to exhaustiveness.
238248
let arm = &self.thir.arms[arm];
239-
self.check_patterns(&arm.pattern, Refutable);
249+
self.with_lint_level(arm.lint_level, |this| {
250+
this.check_patterns(&arm.pattern, Refutable);
251+
});
240252
}
241253

242254
let tarms: Vec<_> = arms

‎tests/ui/lint/lint-attr-everywhere-early.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,14 @@ fn expressions() {
134134
}
135135
}
136136

137+
match f {
138+
#[deny(ellipsis_inclusive_range_patterns)]
139+
Match{f1: 0...100} => {}
140+
//~^ ERROR range patterns are deprecated
141+
//~| WARNING this is accepted in the current edition
142+
_ => {}
143+
}
144+
137145
// Statement Block
138146
{
139147
#![deny(unsafe_code)]

‎tests/ui/lint/lint-attr-everywhere-early.stderr

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -384,103 +384,117 @@ note: the lint level is defined here
384384
LL | #[deny(while_true)]
385385
| ^^^^^^^^^^
386386

387+
error: `...` range patterns are deprecated
388+
--> $DIR/lint-attr-everywhere-early.rs:139:20
389+
|
390+
LL | Match{f1: 0...100} => {}
391+
| ^^^ help: use `..=` for an inclusive range
392+
|
393+
= warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
394+
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
395+
note: the lint level is defined here
396+
--> $DIR/lint-attr-everywhere-early.rs:138:16
397+
|
398+
LL | #[deny(ellipsis_inclusive_range_patterns)]
399+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
400+
387401
error: usage of an `unsafe` block
388-
--> $DIR/lint-attr-everywhere-early.rs:140:9
402+
--> $DIR/lint-attr-everywhere-early.rs:148:9
389403
|
390404
LL | unsafe {}
391405
| ^^^^^^^^^
392406
|
393407
note: the lint level is defined here
394-
--> $DIR/lint-attr-everywhere-early.rs:139:17
408+
--> $DIR/lint-attr-everywhere-early.rs:147:17
395409
|
396410
LL | #![deny(unsafe_code)]
397411
| ^^^^^^^^^^^
398412

399413
error: usage of an `unsafe` block
400-
--> $DIR/lint-attr-everywhere-early.rs:144:9
414+
--> $DIR/lint-attr-everywhere-early.rs:152:9
401415
|
402416
LL | unsafe {}
403417
| ^^^^^^^^^
404418
|
405419
note: the lint level is defined here
406-
--> $DIR/lint-attr-everywhere-early.rs:143:16
420+
--> $DIR/lint-attr-everywhere-early.rs:151:16
407421
|
408422
LL | #[deny(unsafe_code)]
409423
| ^^^^^^^^^^^
410424

411425
error: usage of an `unsafe` block
412-
--> $DIR/lint-attr-everywhere-early.rs:149:5
426+
--> $DIR/lint-attr-everywhere-early.rs:157:5
413427
|
414428
LL | unsafe {};
415429
| ^^^^^^^^^
416430
|
417431
note: the lint level is defined here
418-
--> $DIR/lint-attr-everywhere-early.rs:148:12
432+
--> $DIR/lint-attr-everywhere-early.rs:156:12
419433
|
420434
LL | #[deny(unsafe_code)]
421435
| ^^^^^^^^^^^
422436

423437
error: usage of an `unsafe` block
424-
--> $DIR/lint-attr-everywhere-early.rs:151:27
438+
--> $DIR/lint-attr-everywhere-early.rs:159:27
425439
|
426440
LL | [#[deny(unsafe_code)] unsafe {123}];
427441
| ^^^^^^^^^^^^
428442
|
429443
note: the lint level is defined here
430-
--> $DIR/lint-attr-everywhere-early.rs:151:13
444+
--> $DIR/lint-attr-everywhere-early.rs:159:13
431445
|
432446
LL | [#[deny(unsafe_code)] unsafe {123}];
433447
| ^^^^^^^^^^^
434448

435449
error: usage of an `unsafe` block
436-
--> $DIR/lint-attr-everywhere-early.rs:152:27
450+
--> $DIR/lint-attr-everywhere-early.rs:160:27
437451
|
438452
LL | (#[deny(unsafe_code)] unsafe {123},);
439453
| ^^^^^^^^^^^^
440454
|
441455
note: the lint level is defined here
442-
--> $DIR/lint-attr-everywhere-early.rs:152:13
456+
--> $DIR/lint-attr-everywhere-early.rs:160:13
443457
|
444458
LL | (#[deny(unsafe_code)] unsafe {123},);
445459
| ^^^^^^^^^^^
446460

447461
error: usage of an `unsafe` block
448-
--> $DIR/lint-attr-everywhere-early.rs:154:31
462+
--> $DIR/lint-attr-everywhere-early.rs:162:31
449463
|
450464
LL | call(#[deny(unsafe_code)] unsafe {123});
451465
| ^^^^^^^^^^^^
452466
|
453467
note: the lint level is defined here
454-
--> $DIR/lint-attr-everywhere-early.rs:154:17
468+
--> $DIR/lint-attr-everywhere-early.rs:162:17
455469
|
456470
LL | call(#[deny(unsafe_code)] unsafe {123});
457471
| ^^^^^^^^^^^
458472

459473
error: usage of an `unsafe` block
460-
--> $DIR/lint-attr-everywhere-early.rs:156:38
474+
--> $DIR/lint-attr-everywhere-early.rs:164:38
461475
|
462476
LL | TupleStruct(#[deny(unsafe_code)] unsafe {123});
463477
| ^^^^^^^^^^^^
464478
|
465479
note: the lint level is defined here
466-
--> $DIR/lint-attr-everywhere-early.rs:156:24
480+
--> $DIR/lint-attr-everywhere-early.rs:164:24
467481
|
468482
LL | TupleStruct(#[deny(unsafe_code)] unsafe {123});
469483
| ^^^^^^^^^^^
470484

471485
error: `...` range patterns are deprecated
472-
--> $DIR/lint-attr-everywhere-early.rs:167:18
486+
--> $DIR/lint-attr-everywhere-early.rs:175:18
473487
|
474488
LL | f1: 0...100,
475489
| ^^^ help: use `..=` for an inclusive range
476490
|
477491
= warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
478492
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
479493
note: the lint level is defined here
480-
--> $DIR/lint-attr-everywhere-early.rs:166:20
494+
--> $DIR/lint-attr-everywhere-early.rs:174:20
481495
|
482496
LL | #[deny(ellipsis_inclusive_range_patterns)]
483497
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
484498

485-
error: aborting due to 36 previous errors
499+
error: aborting due to 37 previous errors
486500

‎tests/ui/lint/lint-attr-everywhere-late.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,11 @@ fn expressions() {
162162
}
163163
}
164164

165+
match 123 {
166+
#[deny(non_snake_case)]
167+
ARM_VAR => {} //~ ERROR variable `ARM_VAR` should have a snake case name
168+
}
169+
165170
// Statement Block
166171
{
167172
#![deny(enum_intrinsics_non_enums)]

0 commit comments

Comments
 (0)