Skip to content
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

Fix unexpected_cfgs lint on std #125296

Merged
merged 6 commits into from
May 22, 2024
Merged

Conversation

tesuji
Copy link
Contributor

@tesuji tesuji commented May 19, 2024

closes #125291

r? rust-lang/compiler

@rustbot
Copy link
Collaborator

rustbot commented May 19, 2024

Failed to set assignee to Urgau: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 19, 2024
@Noratrieb
Copy link
Member

Noratrieb commented May 20, 2024

can you find the place in src/bootstrap where it sets the check-cfg for std and attempt to remove it? That would be a simple way to ensure they continue to work.

@Urgau
Copy link
Member

Urgau commented May 20, 2024

This is great, thanks.

can you find the place in src/bootstrap where it sets the check-cfg for std and attempt to remove it?

It won't work for now for this repository as we always build with the previous beta Cargo, and it doesn't know about the check-cfg lint config and even if it did it wouldn't take effect yet. So we must still have the corresponding keys in bootstrap:

(Some(Mode::Std), "stdarch_intel_sde", None),
(Some(Mode::Std), "no_fp_fmt_parse", None),
(Some(Mode::Std), "no_global_oom_handling", None),
(Some(Mode::Std), "no_rc", None),
(Some(Mode::Std), "no_sync", None),
(Some(Mode::Std), "netbsd10", None),
(Some(Mode::Std), "backtrace_in_libstd", None),
/* Extra values not defined in the built-in targets yet, but used in std */
(Some(Mode::Std), "target_env", Some(&["libnx", "p2"])),
(Some(Mode::Std), "target_os", Some(&["visionos"])),
(Some(Mode::Std), "target_arch", Some(&["arm64ec", "spirv", "nvptx", "xtensa"])),

Further more there is currently a warning that is emitted on previous beta Cargo since check-cfg is unused their: warning: /checkout/library/alloc/Cargo.toml: unused manifest key: lints.rust.unexpected_cfgs.check-cfg

We are working on getting this warning removed on beta Cargo, rust-lang/cargo#13925.

@@ -82,6 +82,7 @@ system-llvm-libunwind = ["unwind/system-llvm-libunwind"]

# Make panics and failed asserts immediately abort without formatting any message
panic_immediate_abort = ["core/panic_immediate_abort", "alloc/panic_immediate_abort"]
restricted-std = []
Copy link
Member

@Noratrieb Noratrieb May 20, 2024

Choose a reason for hiding this comment

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

this doesn't really make sense as a feature, using a feature = cfg is a bit weird.. could you put it into the check-cfg table instead of the feature here as well?

Copy link
Member

Choose a reason for hiding this comment

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

If it shouldn't be activated by users, then it shouldn't be Cargo feature to begin with, it should just be a normal "custom cfg": restricted_std.

T-cargo is also thinking of disallowing/warning when expecting a fake Cargo feature, https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/.60check-cfg.60.20and.20custom.20.60feature.60

@Noratrieb
Copy link
Member

I see, makes sense, bootstrapping exists :D. r=me after removing the Cargo feature again.

library/std/Cargo.toml Show resolved Hide resolved

[lints.rust.unexpected_cfgs]
level = "warn"
check-cfg = [
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment saying that these check-cfg entries do not yet take effect for rust-lang/rust since we use beta Cargo, but that for users for -Zbuild-std it does take effect and that the unused warning in waiting for rust-lang/cargo#13925 to reach beta.

library/std/Cargo.toml Show resolved Hide resolved
library/core/Cargo.toml Outdated Show resolved Hide resolved
@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label May 20, 2024
@tesuji
Copy link
Contributor Author

tesuji commented May 20, 2024

@rustbot ready

@michaelwoerister
Copy link
Member

@bors r=Nilstrieb,michaelwoerister

@bors
Copy link
Contributor

bors commented May 21, 2024

📌 Commit 1196df8 has been approved by Nilstrieb,michaelwoerister

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 May 21, 2024
@Noratrieb
Copy link
Member

@bors r=Nilstrieb,michaelwoerister

@bors
Copy link
Contributor

bors commented May 21, 2024

📌 Commit 68a594c has been approved by Nilstrieb,michaelwoerister

It is now in the queue for this repository.

bors added a commit to rust-lang-ci/rust that referenced this pull request May 21, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#124570 (Miscellaneous cleanups)
 - rust-lang#124772 (Refactor documentation for Apple targets)
 - rust-lang#125011 (Add opt-for-size core lib feature flag)
 - rust-lang#125218 (Migrate `run-make/no-intermediate-extras` to new `rmake.rs`)
 - rust-lang#125225 (Use functions from `crt_externs.h` on iOS/tvOS/watchOS/visionOS)
 - rust-lang#125266 (compiler: add simd_ctpop intrinsic)
 - rust-lang#125348 (Small fixes to `std::path::absolute` docs)

Failed merges:

 - rust-lang#125296 (Fix `unexpected_cfgs` lint on std)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented May 21, 2024

☔ The latest upstream changes (presumably #125358) made this pull request unmergeable. Please resolve the merge conflicts.

@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 May 21, 2024
@tesuji tesuji force-pushed the checkcfg-buildstd branch from 68a594c to 30e0ab8 Compare May 21, 2024 18:19
@tesuji

This comment was marked as resolved.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 21, 2024
@michaelwoerister
Copy link
Member

@bors r=Nilstrieb,michaelwoerister

@bors
Copy link
Contributor

bors commented May 21, 2024

📌 Commit 30e0ab8 has been approved by Nilstrieb,michaelwoerister

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 May 21, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
Rollup of 5 pull requests

Successful merges:

 - rust-lang#124896 (miri: rename intrinsic_fallback_checks_ub to intrinsic_fallback_is_spec)
 - rust-lang#125015 (Pattern types: Prohibit generic args on const params)
 - rust-lang#125049 (Disallow cast with trailing braced macro in let-else)
 - rust-lang#125259 (An async closure may implement `FnMut`/`Fn` if it has no self-borrows)
 - rust-lang#125296 (Fix `unexpected_cfgs` lint on std)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8219fd2 into rust-lang:master May 22, 2024
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
Rollup merge of rust-lang#125296 - tesuji:checkcfg-buildstd, r=Nilstrieb,michaelwoerister

Fix `unexpected_cfgs` lint on std

closes rust-lang#125291

r? rust-lang/compiler
@rustbot rustbot added this to the 1.80.0 milestone May 22, 2024
@fmease
Copy link
Member

fmease commented May 22, 2024

bors sleepy @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 May 22, 2024
@tesuji tesuji deleted the checkcfg-buildstd branch May 23, 2024 01:42
@bjorn3
Copy link
Member

bjorn3 commented May 23, 2024

I'm still getting

warning: unexpected `cfg` condition value: `restricted-std`
   --> library/std/src/lib.rs:216:17
    |
216 | #![cfg_attr(not(feature = "restricted-std"), stable(feature = "rust1", since = "1.0.0"))]
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: expected values for `feature` are: `addr2line`, `as_crate`, `backtrace`, `compiler-builtins-c`, `compiler-builtins-mangled-names`, `compiler-builtins-mem`, `compiler-builtins-no-asm`, `compiler-builtins-weak-intrinsics`, `llvm-libunwind`, `miniz_oxide`, `object`, `optimize_for_size`, `panic-unwind`, `panic_immediate_abort`, `panic_unwind`, `profiler`, `profiler_builtins`, `std`, `std_detect_dlsym_getauxval`, `std_detect_env_override`, `std_detect_file_io`, and `system-llvm-libunwind`
    = help: consider adding `restricted-std` as a feature in `Cargo.toml`
    = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
    = note: requested on the command line with `-W unexpected-cfgs`

warning: unexpected `cfg` condition value: `restricted-std`
   --> library/std/src/lib.rs:218:5
    |
218 |     feature = "restricted-std",
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: expected values for `feature` are: `addr2line`, `as_crate`, `backtrace`, `compiler-builtins-c`, `compiler-builtins-mangled-names`, `compiler-builtins-mem`, `compiler-builtins-no-asm`, `compiler-builtins-weak-intrinsics`, `llvm-libunwind`, `miniz_oxide`, `object`, `optimize_for_size`, `panic-unwind`, `panic_immediate_abort`, `panic_unwind`, `profiler`, `profiler_builtins`, `std`, `std_detect_dlsym_getauxval`, `std_detect_env_override`, `std_detect_file_io`, and `system-llvm-libunwind`
    = help: consider adding `restricted-std` as a feature in `Cargo.toml`
    = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration

warning: `std` (lib) generated 2 warnings

@Urgau
Copy link
Member

Urgau commented May 23, 2024

@bjorn3 that's #125296 (comment), I think it would make sense to switch to restricted_std, if it's not supposed to be a Cargo feature.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 23, 2024
… r=bjorn3

Cleanup check-cfg handling in core and std

Follow-up to rust-lang#125296 where we:
 - expect any feature cfg in std, due to `#[path]` imports
 - move some check-cfg args inside the `build.rs` as per Cargo recommendation
 - and replace the fake Cargo feature `"restricted-std"` by the custom cfg `restricted_std`

Fixes rust-lang#125296 (comment)
r? `@bjorn3` (maybe, feel free to re-roll)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 24, 2024
Rollup merge of rust-lang#125452 - Urgau:check-cfg-libraries-cleanup, r=bjorn3

Cleanup check-cfg handling in core and std

Follow-up to rust-lang#125296 where we:
 - expect any feature cfg in std, due to `#[path]` imports
 - move some check-cfg args inside the `build.rs` as per Cargo recommendation
 - and replace the fake Cargo feature `"restricted-std"` by the custom cfg `restricted_std`

Fixes rust-lang#125296 (comment)
r? `@bjorn3` (maybe, feel free to re-roll)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[dogfooding] -Zbuild-std -vv produces lots of warnings about check-cfg
8 participants