Skip to content

Conversation

@clubby789
Copy link
Contributor

@clubby789 clubby789 commented Dec 8, 2025

This implements the followup warning suggested in rust-lang/rfcs#3695
Lint against an empty cfg(any/all), suggest the boolean literal equivalents.
#149791 (comment)

Tracking issue: #131204

@rustbot
Copy link
Collaborator

rustbot commented Dec 8, 2025

Some changes occurred in check-cfg diagnostics

cc @Urgau

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

@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 Dec 8, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 8, 2025

r? @chenyukang

rustbot has assigned @chenyukang.
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

@rust-log-analyzer

This comment has been minimized.

@jdonszelmann
Copy link
Contributor

r? jdonszelmann

@rustbot rustbot assigned jdonszelmann and unassigned chenyukang Dec 8, 2025
@Urgau
Copy link
Member

Urgau commented Dec 8, 2025

Can you split-off the FALSE -> false suggestion into it's own PR?

As for the warning it should probably be a lint (so it can be enabled and disabled), but either way the warning will need to go through T-lang first, with a nomination and maybe a crater run.

@joshtriplett joshtriplett added the I-lang-nominated Nominated for discussion during a lang team meeting. label Dec 8, 2025
@joshtriplett
Copy link
Member

With my T-lang hat on, this seems reasonable. Nominating for discussion of the new lint.

@rust-log-analyzer

This comment has been minimized.

@clubby789
Copy link
Contributor Author

Running into the above error where

    (@__items ($($not:meta,)*) ; ( ($($m:meta),*) ($($tokens:tt)*) ), $($rest:tt)*) => {
        #[cfg(all($($m,)* not(any($($not),*))))] cfg_if! { @__identity $($tokens)* }
        cfg_if! { @__items ($($not,)* $($m,)*) ; $($rest)* }
    };

is invoked with no $not, meaning we expand to not(any()) and raise the lint. Inclined to think that we should only lint for the literal any()/all() cases instead

@rustbot
Copy link
Collaborator

rustbot commented Dec 9, 2025

This PR modifies tests/ui/issues/. If this PR is adding new tests to tests/ui/issues/,
please refrain from doing so, and instead add it to more descriptive subdirectories.

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@clubby789
Copy link
Contributor Author

Implemented as a pre-expansion early lint

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@jdonszelmann jdonszelmann left a comment

Choose a reason for hiding this comment

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

Though I'm not T-lang, I agree with josh that this is a good lint to add. Just got some comments about the implementation but nothing too major. Just gotta clean some things up :)

View changes since this review

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 9, 2025
@bors
Copy link
Collaborator

bors commented Dec 12, 2025

📌 Commit c96ff2d has been approved by jdonszelmann

It is now in the queue for this repository.

@bors bors 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 Dec 12, 2025
@jdonszelmann
Copy link
Contributor

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 12, 2025
@jdonszelmann
Copy link
Contributor

sorry, I do think it's good to change these tests, but could you point me to a test that still tests cfg(all()) and cfg(any()) somewhere? I want to make sure there's still something in ui tests testing the old pattern.

@jdonszelmann
Copy link
Contributor

oh I found it myself, ok my bad
@bors r+

@bors
Copy link
Collaborator

bors commented Dec 12, 2025

📌 Commit c96ff2d has been approved by jdonszelmann

It is now in the queue for this repository.

@bors bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 12, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
…elmann

Remove uses of `cfg({any()/all()})`

~~This implements the followup warning suggested in rust-lang/rfcs#3695
~~Lint against an empty `cfg(any/all)`, suggest the boolean literal equivalents.~~
rust-lang#149791 (comment)

Tracking issue: rust-lang#131204
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
Suggest `cfg(false)` instead of `cfg(FALSE)`

Split from rust-lang#149791

cc `@Urgau`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
…elmann

Remove uses of `cfg({any()/all()})`

~~This implements the followup warning suggested in rust-lang/rfcs#3695
~~Lint against an empty `cfg(any/all)`, suggest the boolean literal equivalents.~~
rust-lang#149791 (comment)

Tracking issue: rust-lang#131204
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
Suggest `cfg(false)` instead of `cfg(FALSE)`

Split from rust-lang#149791

cc ``@Urgau``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
…elmann

Remove uses of `cfg({any()/all()})`

~~This implements the followup warning suggested in rust-lang/rfcs#3695
~~Lint against an empty `cfg(any/all)`, suggest the boolean literal equivalents.~~
rust-lang#149791 (comment)

Tracking issue: rust-lang#131204
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
Suggest `cfg(false)` instead of `cfg(FALSE)`

Split from rust-lang#149791

cc ```@Urgau```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
Suggest `cfg(false)` instead of `cfg(FALSE)`

Split from rust-lang#149791

cc ````@Urgau````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
…elmann

Remove uses of `cfg({any()/all()})`

~~This implements the followup warning suggested in rust-lang/rfcs#3695
~~Lint against an empty `cfg(any/all)`, suggest the boolean literal equivalents.~~
rust-lang#149791 (comment)

Tracking issue: rust-lang#131204
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
Suggest `cfg(false)` instead of `cfg(FALSE)`

Split from rust-lang#149791

cc `````@Urgau`````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
…elmann

Remove uses of `cfg({any()/all()})`

~~This implements the followup warning suggested in rust-lang/rfcs#3695
~~Lint against an empty `cfg(any/all)`, suggest the boolean literal equivalents.~~
rust-lang#149791 (comment)

Tracking issue: rust-lang#131204
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 12, 2025
Suggest `cfg(false)` instead of `cfg(FALSE)`

Split from rust-lang#149791

cc ```````@Urgau```````
bors added a commit that referenced this pull request Dec 12, 2025
Rollup of 7 pull requests

Successful merges:

 - #149655 (bootstrap: add rustc-dev install target)
 - #149789 (Cleanup in the attribute parsers)
 - #149791 (Remove uses of `cfg({any()/all()})`)
 - #149792 (Suggest `cfg(false)` instead of `cfg(FALSE)`)
 - #149883 (add regression test for `proc_macro` error subdiagnostics)
 - #149884 (Clippy subtree update)
 - #149896 (Add myself(makai410) to the review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b826d06 into rust-lang:main Dec 12, 2025
11 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Dec 12, 2025
rust-timer added a commit that referenced this pull request Dec 12, 2025
Rollup merge of #149792 - clubby789:cfg-FALSE, r=jdonszelmann

Suggest `cfg(false)` instead of `cfg(FALSE)`

Split from #149791

cc ```````@Urgau```````
rust-timer added a commit that referenced this pull request Dec 12, 2025
Rollup merge of #149791 - clubby789:cfg-bool-lints, r=jdonszelmann

Remove uses of `cfg({any()/all()})`

~~This implements the followup warning suggested in rust-lang/rfcs#3695
~~Lint against an empty `cfg(any/all)`, suggest the boolean literal equivalents.~~
#149791 (comment)

Tracking issue: #131204
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) I-lang-radar Items that are on lang's radar and will need eventual work or consideration. 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.