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

Stabilize -Zcheck-cfg as always enabled #13571

Merged
merged 3 commits into from
May 3, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Mar 10, 2024

This PR stabilize the -Zcheck-cfg option as always enabled.

Waiting on rust-lang/rust#82450 (comment) to complete, but is otherwise ready to be reviewed (in particular the documentation changes). (rust-lang/rust#123501)

Fixes #10554

@rustbot
Copy link
Collaborator

rustbot commented Mar 10, 2024

r? @ehuss

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

@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts A-documenting-cargo-itself Area: Cargo's documentation A-rebuild-detection Area: rebuild detection and fingerprinting A-unstable Area: nightly unstable support S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 10, 2024
src/cargo/core/features.rs Outdated Show resolved Hide resolved
@bors
Copy link
Collaborator

bors commented Mar 23, 2024

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

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 3, 2024
@bors
Copy link
Collaborator

bors commented May 3, 2024

⌛ Testing commit 388a17f with merge 7ebc065...

@bors
Copy link
Collaborator

bors commented May 3, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 7ebc065 to master...

@bors bors merged commit 7ebc065 into rust-lang:master May 3, 2024
21 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
Update cargo

18 commits in 6087566b3fa73bfda29702632493e938b12d19e5..05364cb2f61a2c2b091e061c1f42b207dfb5f81f
2024-04-30 20:45:20 +0000 to 2024-05-03 16:48:59 +0000
- chore(deps): update msrv (3 versions) to v1.76 (rust-lang/cargo#13857)
- Stabilize `-Zcheck-cfg` as always enabled (rust-lang/cargo#13571)
- fix(lints): Prevent inheritance from bring exposed for published packages (rust-lang/cargo#13852)
- refactor: remove unnecessary branch for link binary on macOS (rust-lang/cargo#13851)
- perf(toml): Avoid inferring when targets are known (rust-lang/cargo#13849)
- Update continuous-integration.md: Include CircleCI reference (rust-lang/cargo#13850)
- chore(deps): update msrv (1 version) to v1.78 (rust-lang/cargo#13848)
- Workaround copying file returning EAGAIN on ZFS on mac OS (rust-lang/cargo#13845)
- Clean package perf improvements (rust-lang/cargo#13818)
- fix(toml): Validate crates_types/proc-macro for bin like others (rust-lang/cargo#13841)
- fix(toml): On 2024 Edition, disallow ignored `default-features` when inheriting (rust-lang/cargo#13839)
- chore(ci): Ignore openssl deps (rust-lang/cargo#13840)
- fix(lints): Remove ability to specify `-` in lint name (rust-lang/cargo#13837)
- fix(resolver): Treat unset MSRV as compatible (rust-lang/cargo#13791)
- fix(toml): Don't lose 'public' when inheriting a dep (rust-lang/cargo#13836)
- chore(deps): update compatible (rust-lang/cargo#13834)
- Error when unstable lints are specified but not enabled (rust-lang/cargo#13805)
- fix(lint): Warn not Error on unsupported lint tool (rust-lang/cargo#13833)

r? ghost

Note: this includes the fix that was beta backported in rust-lang#124647
bors added a commit to rust-lang-ci/rust that referenced this pull request May 4, 2024
Update cargo

18 commits in 6087566b3fa73bfda29702632493e938b12d19e5..05364cb2f61a2c2b091e061c1f42b207dfb5f81f
2024-04-30 20:45:20 +0000 to 2024-05-03 16:48:59 +0000
- chore(deps): update msrv (3 versions) to v1.76 (rust-lang/cargo#13857)
- Stabilize `-Zcheck-cfg` as always enabled (rust-lang/cargo#13571)
- fix(lints): Prevent inheritance from bring exposed for published packages (rust-lang/cargo#13852)
- refactor: remove unnecessary branch for link binary on macOS (rust-lang/cargo#13851)
- perf(toml): Avoid inferring when targets are known (rust-lang/cargo#13849)
- Update continuous-integration.md: Include CircleCI reference (rust-lang/cargo#13850)
- chore(deps): update msrv (1 version) to v1.78 (rust-lang/cargo#13848)
- Workaround copying file returning EAGAIN on ZFS on mac OS (rust-lang/cargo#13845)
- Clean package perf improvements (rust-lang/cargo#13818)
- fix(toml): Validate crates_types/proc-macro for bin like others (rust-lang/cargo#13841)
- fix(toml): On 2024 Edition, disallow ignored `default-features` when inheriting (rust-lang/cargo#13839)
- chore(ci): Ignore openssl deps (rust-lang/cargo#13840)
- fix(lints): Remove ability to specify `-` in lint name (rust-lang/cargo#13837)
- fix(resolver): Treat unset MSRV as compatible (rust-lang/cargo#13791)
- fix(toml): Don't lose 'public' when inheriting a dep (rust-lang/cargo#13836)
- chore(deps): update compatible (rust-lang/cargo#13834)
- Error when unstable lints are specified but not enabled (rust-lang/cargo#13805)
- fix(lint): Warn not Error on unsupported lint tool (rust-lang/cargo#13833)

r? ghost

Note: this includes the fix that was beta backported in rust-lang#124647
@rustbot rustbot added this to the 1.80.0 milestone May 4, 2024
weihanglo added a commit to weihanglo/cargo that referenced this pull request May 4, 2024
…weihanglo"

This reverts commit 7ebc065, reversing
changes made to cf7b3c4.
@Darksonn
Copy link

Darksonn commented May 5, 2024

Would it make sense to add some common --cfg names from the ecosystem such as #[cfg(loom)] to the allow-list?

@epage
Copy link
Contributor

epage commented May 6, 2024

Its a bit difficult to decide what is "common" enough to justify inclusion and what level of compatibility we should provide on these. For now, we are sticking with cfg's that are first-party to cargo.

Something like https://internals.rust-lang.org/t/pre-rfc-mutually-excusive-global-features/19618 would allow registering more configs with Cargo in a declarative manner.

It is unfortunate that hyper and tokio are disabling the lint to avoid the overhead of build scripts

Tokio at least has a custom CI job where they check it. I guess another alternative is a config file with the rustflags set though I always find that annoying when contributing to repos due to my personal workflow.

@Darksonn
Copy link

Darksonn commented May 6, 2024

I understand that including third-party --cfgs is a bit of a can of worms. But I also think that it would significantly reduce the amount of crates that are going to have to silence the lint. You could consider doing a crater run to figure out which flags are common?

In Tokio's case, there's probably no way around disabling it. We use too many --cfg flags for various bespoke ways of configuring the test suite. Making them into features will not work. Build scripts hurt compile times for end users, and rustflags is a bad experience for new contributors. I think that having a dedicated CI run that enables the lint is reasonable compromise.

@Urgau
Copy link
Member Author

Urgau commented May 6, 2024

You could consider doing a crater run to figure out which flags are common?

We did, you can see the results here: rust-lang/rust#120701 (comment)

Deciding what is common enough is difficult since every use-case can be vastly different, the Rust-for-Linux project probably don't want your loom cfg in their build system.

In the stabilization report of --check-cfg I proposed a simple rule for what is constitute a well known cfg, it must be set by a Rust Toolchain tool since that is the least common denominator for all users, everything else is not a well known cfg.

@kennykerr
Copy link

kennykerr commented May 6, 2024

Looks like I'll also have to silence the warning: microsoft/windows-rs#3022

@Thomasdezeeuw
Copy link

I want to reiterate that this creates an issue and a lot of churn for crates that have valid cfg attributes outside of the "expected" list.

Would it be possible for the code itself to define a list of correct (additional) attributes? To give a concrete example, Mio uses two cfg attributes to force a specific implementations. It would be nice if we could add something like the following to our code so that Rust can safe ignore the warnings.

#![expected_cfgs(mio_unsupported_force_poll_poll, mio_unsupported_force_waker_pipe)]

I think this would also solve the "common third party" issue as Loom (and other crates) could recommend to put something like the above into the users code.

@ChrisDenton
Copy link
Contributor

See also rust-lang/rust#124800. It's probably best to keep the discussion in one place.

flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 24, 2024
Update cargo

18 commits in 6087566b3fa73bfda29702632493e938b12d19e5..05364cb2f61a2c2b091e061c1f42b207dfb5f81f
2024-04-30 20:45:20 +0000 to 2024-05-03 16:48:59 +0000
- chore(deps): update msrv (3 versions) to v1.76 (rust-lang/cargo#13857)
- Stabilize `-Zcheck-cfg` as always enabled (rust-lang/cargo#13571)
- fix(lints): Prevent inheritance from bring exposed for published packages (rust-lang/cargo#13852)
- refactor: remove unnecessary branch for link binary on macOS (rust-lang/cargo#13851)
- perf(toml): Avoid inferring when targets are known (rust-lang/cargo#13849)
- Update continuous-integration.md: Include CircleCI reference (rust-lang/cargo#13850)
- chore(deps): update msrv (1 version) to v1.78 (rust-lang/cargo#13848)
- Workaround copying file returning EAGAIN on ZFS on mac OS (rust-lang/cargo#13845)
- Clean package perf improvements (rust-lang/cargo#13818)
- fix(toml): Validate crates_types/proc-macro for bin like others (rust-lang/cargo#13841)
- fix(toml): On 2024 Edition, disallow ignored `default-features` when inheriting (rust-lang/cargo#13839)
- chore(ci): Ignore openssl deps (rust-lang/cargo#13840)
- fix(lints): Remove ability to specify `-` in lint name (rust-lang/cargo#13837)
- fix(resolver): Treat unset MSRV as compatible (rust-lang/cargo#13791)
- fix(toml): Don't lose 'public' when inheriting a dep (rust-lang/cargo#13836)
- chore(deps): update compatible (rust-lang/cargo#13834)
- Error when unstable lints are specified but not enabled (rust-lang/cargo#13805)
- fix(lint): Warn not Error on unsupported lint tool (rust-lang/cargo#13833)

r? ghost

Note: this includes the fix that was beta backported in #124647
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts A-cfg-expr Area: Platform cfg expressions A-documenting-cargo-itself Area: Cargo's documentation A-rebuild-detection Area: rebuild detection and fingerprinting A-unstable Area: nightly unstable support S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking Issue for rustc --check-cfg integration
10 participants