Skip to content

Pre lint port cleanups - #162813

Merged
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
mejrs:pre_lint_cleanups
Sep 16, 2026
Merged

rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
mejrs:pre_lint_cleanups

Conversation

@mejrs

@mejrs mejrs commented Sep 15, 2026

Copy link
Copy Markdown
Member

Some minor changes split off from #162811, to make that a bit easier to review.

cc @JonathanBrouwer

@rustbot

rustbot commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 15, 2026
@rustbot

rustbot commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

r? @JonathanBrouwer

rustbot has assigned @JonathanBrouwer.
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 76 candidates
  • Random selection from 18 candidates

#[expect[wut]] // OK
#[expect(expect)] // OK
#[expect(expect(expect))] //~ ERROR malformed lint attribute input
//~| ERROR malformed lint attribute input

@petrochenkov petrochenkov Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also should be ok.

Before the const _ item is fully expanded (the item itself, not its nested nodes), all the #[expect]s (and other potential inert attributes) are just tokens without any semantic meaning, and the active attributes can change those tokens in any way or remove them like cfg(false) does, so any semantic checks like "this built-in attribute has unexpected input" should not be performed for them.

View changes since the review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this sense

#[expect(expect(expect))]
expand_to_nothing!();

is also equivalent to #[expect(expect(expect))] in cfg(false) code, and also shouldn't report any "malformed" attribute errors, and #160904 is not doing the right thing.
Although it would still be nice to conservatively turn it into an error, and then try turning #[attrs] in #[attrs] mac_call!() into a part of the macro input (#63221 (comment)).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also should be ok.

I agree, it's annoyingly inconsistent. I'm just adding the test here to document the current behavior and I don't want to change that behavior before or during #162811 because it'll make porting lint attributes much harder to write and review.

Before the const _ item is fully expanded (the item itself, not its nested nodes), all the #[expect]s (and other potential inert attributes) are just tokens without any semantic meaning ...

They have meaning for pre-expansion lints, so it's not as simple as "they're just tokens":

//@ edition: 2015

#[warn(keyword_idents_2024)]
#[cfg(false)]
fn foo() {
    let gen = 5; //~ WARN `gen` is a keyword in the 2024 edition
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since when the example with #[warn(keyword_idents_2024)] works, and how?
Previously lint attributes were only attached to node ids, and node ids are not created for cfg(false) code.
I don't think it's generally a good idea to support this, if it goes against the core expansion model.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to change that behavior before or during #162811 because it'll make porting lint attributes much harder to write and review.

With this I agree.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This equivalent example works in rust 1.30:

//@ edition: 2015

#[warn(keyword_idents)]
#[cfg(any())]
fn foo() {
    let async = 5; // `async` is a keyword in the 2018 edition
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, weird, I need to investigate.

@petrochenkov petrochenkov removed their assignment Sep 15, 2026
@petrochenkov

Copy link
Copy Markdown
Contributor

r=me

@JonathanBrouwer

Copy link
Copy Markdown
Member

I'd like to take a look at this as well, can probably make some time tonight

@JonathanBrouwer JonathanBrouwer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors r=JonathanBrouwer,petrochenkov rollup

View changes since this review

@rust-bors

rust-bors Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 6d505c6 has been approved by JonathanBrouwer,petrochenkov

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 15, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 16, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #161873 (move bug and span_bug macros to rustc_span)
 - #162584 (Use File::*lock*() in rustc_data_structures::flock when possible)
 - #162704 (Parser: Uninterpolate when checking for const closures, try bikeshed blocks & in relevant diagnostic code)
 - #162787 (Fix suggestions for names captured by formatting macros)
 - #159562 (Windows: cache the stdio write mode for the duration of a lock session)
 - #162813 (Pre lint port cleanups)
 - #162822 (PassWrapper: adapt for removal of EABIVersion arg in LLVM 24)
@rust-bors
rust-bors Bot merged commit 354f732 into rust-lang:main Sep 16, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Sep 16, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 16, 2026
Rollup merge of #162813 - mejrs:pre_lint_cleanups, r=JonathanBrouwer,petrochenkov

Pre lint port cleanups

Some minor changes split off from #162811, to make that a bit easier to review.

cc @JonathanBrouwer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants