[mejrs]

In porting the lint attributes, it also added a fcw if they're used on a macro invocation. However t is something that has meaning, so revert the FCW.

// edition ..2021
macro_rules! wat {
    ($($tt:tt)*) => {}
}

fn main(){
    #[deny(keyword_idents_2024)]
    wat!(gen);
    //~^ ERROR `gen` is a keyword in the 2024 edition
    //~| WARN this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024!
}

So I don't think this should be a fcw.

Similarly, clippy currently does this:

fn main() {
    #[deny(clippy::nonstandard_macro_braces)]
    println! {};
}

error: use of irregular braces for `println!` macro
 --> src/main.rs:3:5
  |
3 |     println! {};
  |     ^^^^^^^^^^^ help: consider writing: `println!()`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/main/index.html#nonstandard_macro_braces
note: the lint level is defined here
 --> src/main.rs:2:12
  |
2 |     #[deny(clippy::nonstandard_macro_braces)]
  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: the `deny` attribute cannot be used on macro calls
 --> src/main.rs:2:7
  |
2 |     #[deny(clippy::nonstandard_macro_braces)]
  |       ^^^^
3 |     println! {};
  |
  = help: the `deny` attribute can be applied to associated consts, associated types, break expressions, const parameters, constants, crates, data types, enum variants, extern crates, for loops, foreign modules, foreign statics, function params, functions, global asms, impl blocks, lifetime parameters, loops, macro defs, match arms, modules, pattern fields, statics, struct fields, trait aliases, traits, type aliases, type parameters, use statements, and while loops
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: placing this attribute on a macro invocation does nothing even if the macro expands to what would be a valid target for the attribute
  = note: `#[warn(unused_attributes)]` (part of `#[warn(unused)]`) on by default

[rustbot]

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

[rustbot]

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 77 candidates
• Random selection from 20 candidates

[JonathanBrouwer]

I think it makes sense to emit the warning iff none of the lints that are configured are pre-expansion lints. I'm not sure how easy that would be to implement tho...

View changes since the review

[mejrs]

We do have some cases where this is linted against based on the lint, like non_exhaustive_omitted_patterns on match arms for example.

[mejrs]

(It would be nice to get this in before the beta cutoff, otherwise it will require backporting)

[JonathanBrouwer]

I think since this is a change to when a lint is emitted, the lang team needs to approve this.
If you'd like to get the specific regression fixed quickly, might be better to special-case this not to emit the FCW, but still emit an unused attributes warning.

[mejrs]

I think since this is a change to when a lint is emitted, the lang team needs to approve this.

It was added in #87296, without lang involvement. The FCW was added to it after I ported the lint attributes, again without lang involvement.

Both were probably mistakes. and the correct process would be to revert both and then (if desired) land it again with t-lang approval.

[JonathanBrouwer]

Since the lint has already been on stable for 5 years, and it is useful in most cases, I don't like removing it entirely, especially not without T-lang approval.

For example, the lint helps identify that lint attributes are not applied to the expanded code of a macro:

macro_rules! nice {
    () => {
        // mhuahuahua :D
        unsafe { *std::ptr::null::<()>() } //~ OK
    };
}

fn main(){
    #[deny(unsafe_code)]
    nice!();
}

(I'm still happy to revert the FCW part)

[mejrs]

Sounds good, I'll do that.

[mejrs]

I think it would generally be a good idea to do a future PR to emit a lint for "this particular lint on this target doesn't make sense"

[JonathanBrouwer]

I think it would generally be a good idea to do a future PR to emit a lint for "this particular lint on this target doesn't make sense"

Sounds like a good idea!

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[JonathanBrouwer]

@bors r+ rollup

View changes since this review

[rust-bors]

📌 Commit 6c7f841 has been approved by JonathanBrouwer

It is now in the queue for this repository.