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

register_late_mod_pass doesn't visit all attributes #115571

Closed
Alexendoo opened this issue Sep 5, 2023 · 0 comments · Fixed by #115739
Closed

register_late_mod_pass doesn't visit all attributes #115571

Alexendoo opened this issue Sep 5, 2023 · 0 comments · Fixed by #115739

Comments

@Alexendoo
Copy link
Member

Alexendoo commented Sep 5, 2023

I was testing using per module passes (like #113734) in Clippy and saw test failures when trying clippy::allow_attributes, it seems only inner attributes of the root crate are visited by per module late lint passes

Reproducer without having to compile clippy:

diff --git i/compiler/rustc_lint/src/for_loops_over_fallibles.rs w/compiler/rustc_lint/src/for_loops_over_fallibles.rs
index c299e38842a..47595bcba77 100644
--- i/compiler/rustc_lint/src/for_loops_over_fallibles.rs
+++ w/compiler/rustc_lint/src/for_loops_over_fallibles.rs
@@ -80,6 +80,10 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
             ForLoopsOverFalliblesDiag { article, ty, sub, question_mark, suggestion },
         );
     }
+
+    fn check_attribute(&mut self, _: &LateContext<'_>, attr: &'_ rustc_ast::Attribute) {
+        dbg!(attr.span);
+    }
 }
 
 fn extract_for_loop<'tcx>(expr: &Expr<'tcx>) -> Option<(&'tcx Pat<'tcx>, &'tcx Expr<'tcx>)> {

Compile + run rustc on

#![warn(clippy::a)]

#[warn(clippy::b)]
pub struct B;

pub mod m {
    #![warn(clippy::c)]

    #[warn(clippy::d)]
    pub struct D;
}

fn main() {}

You'll see only one attribute in the output

[compiler/rustc_lint/src/for_loops_over_fallibles.rs:85] attr.span = foo.rs:1:1: 1:20 (#0)
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 5, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 11, 2023
…e, r=oli-obk

Call `LateLintPass::check_attribute` from `with_lint_attrs`

Fixes rust-lang#115571

For regular `register_late_pass` lints also means that `last_node_with_lint_attrs` is correct when in `check_attribute`, I've added a test that previously failed for `clippy::allow_attributes`

As far as I can see the only late lint in rustc that uses `check_attribute` is `unstable_features` which is allow by default and deprecated so this is mostly for clippy (or future rustc lints)
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Sep 11, 2023
…e, r=oli-obk

Call `LateLintPass::check_attribute` from `with_lint_attrs`

Fixes rust-lang#115571

For regular `register_late_pass` lints also means that `last_node_with_lint_attrs` is correct when in `check_attribute`, I've added a test that previously failed for `clippy::allow_attributes`

As far as I can see the only late lint in rustc that uses `check_attribute` is `unstable_features` which is allow by default and deprecated so this is mostly for clippy (or future rustc lints)
@bors bors closed this as completed in d24f575 Sep 11, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 11, 2023
Rollup merge of rust-lang#115739 - Alexendoo:lint-pass-check-attribute, r=oli-obk

Call `LateLintPass::check_attribute` from `with_lint_attrs`

Fixes rust-lang#115571

For regular `register_late_pass` lints also means that `last_node_with_lint_attrs` is correct when in `check_attribute`, I've added a test that previously failed for `clippy::allow_attributes`

As far as I can see the only late lint in rustc that uses `check_attribute` is `unstable_features` which is allow by default and deprecated so this is mostly for clippy (or future rustc lints)
@apiraino apiraino removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants