Auto merge of #85312 - ehuss:macro_use-unused-attr, r=petrochenkov
Fix unused attributes on macro_rules. The `unused_attributes` lint wasn't firing on attributes of `macro_rules` definitions. The consequence is that many attributes are silently ignored on `macro_rules`. The reason is that `unused_attributes` is a late-lint pass, and only has access to the HIR, which does not have macro_rules definitions. My solution here is to change `non_exported_macro_attrs` to be `macro_attrs` (a list of all attributes used for `macro_rules`, instead of just those for `macro_export`), and then to check this list in the `unused_attributes` lint. There are a number of alternate approaches, but this seemed the most reliable and least invasive. I am open to completely different approaches, though. One concern is that I don't fully understand the implications of extending `non_exported_macro_attrs` to include non-exported macros. That list was originally added in #62042 to handle stability attributes, so I suspect it was just an optimization since that was all that was needed. It was later extended to be included in SVH in #83901. #80641 also added a use to check for `invalid` attributes, which seems a little odd to me (it didn't validate non-exported macros, and seems highly specific). Overall, there doesn't seem to be a clear story of when `unused_attributes` should be used versus an error like E0518. I considered alternatively using an "allow list" of built-in attributes that can be used on macro_rules (allow, warn, deny, forbid, cfg, cfg_attr, macro_export, deprecated, doc), but I feel like that could be a pain to maintain. Some built-in attributes already present hard-errors when used with macro_rules. These are each hard-coded in various places: - `derive` - `test` and `bench` - `proc_macro` and `proc_macro_derive` - `inline` - `global_allocator` The primary motivation is that I sometimes see people use `#[macro_use]` in front of `macro_rules`, which indicates there is some confusion out there (evident that there was even a case of it in rustc).
This commit is contained in:
commit
fe72845f7b
4 changed files with 70 additions and 3 deletions
|
@ -448,6 +448,10 @@ fn late_lint_pass_crate<'tcx, T: LateLintPass<'tcx>>(tcx: TyCtxt<'tcx>, pass: T)
|
|||
lint_callback!(cx, check_crate, krate);
|
||||
|
||||
hir_visit::walk_crate(cx, krate);
|
||||
for attr in krate.non_exported_macro_attrs {
|
||||
// This HIR ID is a lie, since the macro ID isn't available.
|
||||
cx.visit_attribute(hir::CRATE_HIR_ID, attr);
|
||||
}
|
||||
|
||||
lint_callback!(cx, check_crate_post, krate);
|
||||
})
|
||||
|
|
|
@ -6,7 +6,6 @@ use rustc_span::Symbol;
|
|||
use std::fmt;
|
||||
use std::str::FromStr;
|
||||
|
||||
#[macro_use]
|
||||
macro_rules! def_reg_class {
|
||||
($arch:ident $arch_regclass:ident {
|
||||
$(
|
||||
|
@ -51,7 +50,6 @@ macro_rules! def_reg_class {
|
|||
}
|
||||
}
|
||||
|
||||
#[macro_use]
|
||||
macro_rules! def_regs {
|
||||
($arch:ident $arch_reg:ident $arch_regclass:ident {
|
||||
$(
|
||||
|
@ -129,7 +127,6 @@ macro_rules! def_regs {
|
|||
}
|
||||
}
|
||||
|
||||
#[macro_use]
|
||||
macro_rules! types {
|
||||
(
|
||||
$(_ : $($ty:expr),+;)?
|
||||
|
|
34
src/test/ui/unused/unused-attr-macro-rules.rs
Normal file
34
src/test/ui/unused/unused-attr-macro-rules.rs
Normal file
|
@ -0,0 +1,34 @@
|
|||
#![deny(unused_attributes)]
|
||||
// Unused attributes on macro_rules requires special handling since the
|
||||
// macro_rules definition does not survive towards HIR.
|
||||
|
||||
// A sample of various built-in attributes.
|
||||
#[macro_export]
|
||||
#[macro_use] //~ ERROR unused attribute
|
||||
#[path="foo"] //~ ERROR unused attribute
|
||||
#[recursion_limit="1"] //~ ERROR unused attribute
|
||||
//~| ERROR crate-level attribute should be an inner attribute
|
||||
macro_rules! foo {
|
||||
() => {};
|
||||
}
|
||||
|
||||
// The following should not warn about unused attributes.
|
||||
#[allow(unused)]
|
||||
macro_rules! foo2 {
|
||||
() => {};
|
||||
}
|
||||
|
||||
#[cfg(FALSE)]
|
||||
macro_rules! foo {
|
||||
() => {};
|
||||
}
|
||||
|
||||
/// Some docs
|
||||
#[deprecated]
|
||||
#[doc = "more docs"]
|
||||
#[macro_export]
|
||||
macro_rules! bar {
|
||||
() => {};
|
||||
}
|
||||
|
||||
fn main() {}
|
32
src/test/ui/unused/unused-attr-macro-rules.stderr
Normal file
32
src/test/ui/unused/unused-attr-macro-rules.stderr
Normal file
|
@ -0,0 +1,32 @@
|
|||
error: unused attribute
|
||||
--> $DIR/unused-attr-macro-rules.rs:7:1
|
||||
|
|
||||
LL | #[macro_use]
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
note: the lint level is defined here
|
||||
--> $DIR/unused-attr-macro-rules.rs:1:9
|
||||
|
|
||||
LL | #![deny(unused_attributes)]
|
||||
| ^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: unused attribute
|
||||
--> $DIR/unused-attr-macro-rules.rs:8:1
|
||||
|
|
||||
LL | #[path="foo"]
|
||||
| ^^^^^^^^^^^^^
|
||||
|
||||
error: unused attribute
|
||||
--> $DIR/unused-attr-macro-rules.rs:9:1
|
||||
|
|
||||
LL | #[recursion_limit="1"]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: crate-level attribute should be an inner attribute: add an exclamation mark: `#![foo]`
|
||||
--> $DIR/unused-attr-macro-rules.rs:9:1
|
||||
|
|
||||
LL | #[recursion_limit="1"]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 4 previous errors
|
||||
|
Loading…
Add table
Reference in a new issue