-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add clippy into the known cfg
list
#121137
Conversation
@@ -1441,6 +1441,7 @@ impl CheckCfg { | |||
// These three are never set by rustc, but we set them anyway: they | |||
// should not trigger a lint because `cargo doc`, `cargo test`, and | |||
// `cargo miri run` (respectively) can set them. | |||
ins!(sym::clippy, no_values); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to be added in tests/ui/check-cfg/well-known-values.rs.
You will also probably need to bless the directory
This comment has been minimized.
This comment has been minimized.
@@ -1441,6 +1441,7 @@ impl CheckCfg { | |||
// These three are never set by rustc, but we set them anyway: they | |||
// should not trigger a lint because `cargo doc`, `cargo test`, and | |||
// `cargo miri run` (respectively) can set them. | |||
ins!(sym::clippy, no_values); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check-cfg section of the unstable book also needs updating:
rust/src/doc/unstable-book/src/compiler-flags/check-cfg.md
Lines 80 to 87 in 4ae1e79
As of `2024-01-09T`, the list of known names is as follows: | |
<!--- See CheckCfg::fill_well_known in compiler/rustc_session/src/config.rs --> | |
- `debug_assertions` | |
- `doc` | |
- `doctest` | |
- `miri` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code here should have a comment mentioning the docs :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding it.
150cef9
to
2bffcbf
Compare
@@ -81,6 +81,7 @@ As of `2024-01-09T`, the list of known names is as follows: | |||
|
|||
<!--- See CheckCfg::fill_well_known in compiler/rustc_session/src/config.rs --> | |||
|
|||
- `clippy` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great if you could also update the date above to 2024-02-15T
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -106,4 +106,7 @@ fn unix() {} | |||
#[cfg(doc)] | |||
fn doc() {} | |||
|
|||
#[cfg(clippy)] | |||
fn clippy() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add the cfg above as to check that we only expect clippy
and nothing else.
#[cfg(any(
// tidy-alphabetical-start
+ clippy = "_UNEXPECTED_VALUE",
+ //~^ WARN unexpected `cfg` condition value
debug_assertions = "_UNEXPECTED_VALUE",
//~^ WARN unexpected `cfg` condition value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sure.
2bffcbf
to
a1c10e7
Compare
Updated! Thanks a lot for the reviews! |
This comment has been minimized.
This comment has been minimized.
a1c10e7
to
870127e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks.
compiler/rustc_session/src/config.rs
Outdated
// should not trigger a lint because `cargo doc`, `cargo test`, | ||
// `cargo clippy` and `cargo miri run` (respectively) can set them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
// should not trigger a lint because `cargo doc`, `cargo test`, | |
// `cargo clippy` and `cargo miri run` (respectively) can set them. | |
// should not trigger a lint because `cargo clippy`, `cargo doc`, | |
// `cargo test` and `cargo miri run` (respectively) can set them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, let's alpha sort them. 😆
870127e
to
4085421
Compare
…r if the check-cfg list is updated
Guillaume asked me to review the PR since I commented on it, and it looks good to me. |
@bors rollup |
…llaumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#119928 (suggest `into_iter()` when `Iterator` method called on `impl IntoIterator`) - rust-lang#121020 (Avoid an ICE in diagnostics) - rust-lang#121111 (For E0038, suggest associated type if available) - rust-lang#121137 (Add clippy into the known `cfg` list) - rust-lang#121179 (allow mutable references in const values when they point to no memory) - rust-lang#121181 (Fix an ICE in the recursion lint) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#121137 - GuillaumeGomez:add-clippy-cfg, r=Urgau,Nilstrieb Add clippy into the known `cfg` list In clippy, we are removing the `feature = "cargo-clippy"` cfg to replace it with `clippy` in rust-lang/rust-clippy#12292. But for it to work, we need to declare `clippy` as cfg. It makes it more coherent with other existing tools like rustdoc. cc `@flip1995`
In clippy, we are removing the
feature = "cargo-clippy"
cfg to replace it withclippy
in rust-lang/rust-clippy#12292. But for it to work, we need to declareclippy
as cfg. It makes it more coherent with other existing tools like rustdoc.cc @flip1995