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

Tracking Issue for rustc --check-cfg integration #10554

Closed
1 of 2 tasks
weihanglo opened this issue Apr 11, 2022 · 33 comments · Fixed by #13571
Closed
1 of 2 tasks

Tracking Issue for rustc --check-cfg integration #10554

weihanglo opened this issue Apr 11, 2022 · 33 comments · Fixed by #13571
Labels
C-tracking-issue Category: A tracking issue for something unstable. disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. T-cargo Team: Cargo to-announce
Projects

Comments

@weihanglo
Copy link
Member

weihanglo commented Apr 11, 2022

Summary

RFC: #3013
rustc tracking issue: rust-lang/rust#82450

Documentation:

This tracks the integration of rustc --check-cfg flag (RFC #3013), which enables complete or partial checking of conditional compilation configuration at compile time.

Unresolved Issues

  • Should these flags be enabled by default? What's the unintrusive interface to provide for users to opt-out the checking?
  • If we feel the diagnostic on using a feature when no features are defined is insufficient, then do we feel it would not be incongruous with the design to fix this in the future? (see Fix --check-cfg invocations with zero features #13011).

Post-RFC decisions

  • Support cargo:rustc-check-cfg despite :: being introduced in 1.77 and this being released after that. This was done to allow for wider MSRVs while using this feature

Future Extensions

No response

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Implementation history:

@est31
Copy link
Member

est31 commented Jun 14, 2022

Over in rust-lang/rust#93628 I've discovered two things:

  • First, a papercut: apparently there are empty values() calls when the feature is enabled, even if there is another values(...) call with content present already. In that case, values() is not needed, right?
  • Second, due to There is no way to set RUSTFLAGS for build scripts when cross-compiling #4423 specifying custom --check-cfg inside RUSTFLAGS does not reach all of cargo's units, as in, there is no way to specify custom flags for the host units. Apparently it's still an open question how one should enable people to specify custom possible cfg names, but the way bootstrap does it via RUSTFLAGS is IMO not enough to get --check-cfg stabilized. Even if a user-friendly way via toml editing exists, ideally there would also be a way that can be set programmatically via some kind of environment variable. edit: resolved, see Tracking Issue for rustc --check-cfg integration #10554 (comment)

@Urgau
Copy link
Contributor

Urgau commented Jun 15, 2022

* First, a papercut: apparently there are empty `values()` calls when the feature is enabled, even if there is another `values(...)` call with content present already. In that case, `values()` is not needed, right?

You're mixing things, bootstrap (the tool that build rustc, rustdoc, ...) use cargo -Zcheck-cfg=names,values,features to enable:

  • well known names: --check-cfg=names()
  • well known values: --check-cfg=values()
  • well known features: --check-cfg=values(feature, ...)

And then use RUSTFLAGS to set extra check cfg (most of them will go any when cargo beta has support for rustc-check-cfg in build script). Cargo is not involved at all in this process so if there are duplicates, nothing can be done. This explain what you see.

Concerning empty values() the unstable book as a section about it check-cfg values:
     If values() is specified, then rustc will enable the checking of well-known values defined by itself. Note that it's necessary to specify the values() form to enable the checking of well known values, specifying the other forms doesn't implicitly enable it.

Btw, this should have been posted on the rust-lang/rust Tracking Issue for RFC 3013: Checking conditional compilation at compile time.

* Second, due to [There is no way to set RUSTFLAGS for build scripts when cross-compiling #4423](https://github.com/rust-lang/cargo/issues/4423) specifying custom `--check-cfg` inside `RUSTFLAGS` does not reach all of cargo's units, as in, there is no way to specify custom flags for the host units. Apparently it's [still an open question](https://github.com/rust-lang/rust/issues/82450#issuecomment-1046819607) how one should enable people to specify custom possible cfg names, but the way bootstrap does it via `RUSTFLAGS` is IMO not enough to get `--check-cfg` stabilized. Even if a user-friendly way via toml editing exists, ideally there would also be a way that can be set programmatically via some kind of environment variable.

Why would this be a concern for stabilization ? I don't think it's that big of a deal, bootstrap and the rust-lang/rust codebase is very special, the vast majority of project who would use check-cfg (which is less probably than 1%) would simply add rustc-check-cfg to their build script or in their Cargo.toml. I don't think anyone would use RUSTFLAGS for this.

Nevertheless I agree that I would be great if there was a way to forward RUSTFLAGS to proc-macro when --target is specified.

@est31
Copy link
Member

est31 commented Jun 15, 2022

Concerning empty values() the unstable book as a section about it check-cfg values:

Oh I see, that's a weird choice that IMO violates the least surprise principle. I've thought that this was some genuine bug of cargo's integration of --check-cfg. I guess how that feature can be improved is indeed discussed in the rustc tracking issue. But it's a papercut, shrug.

the vast majority of project who would use check-cfg (which is less probably than 1%) would simply add rustc-check-cfg to their build script or in their Cargo.toml. I don't think anyone would use RUSTFLAGS for this.

My original reasoning was that there is a use case of wanting to enable some feature that needs different code across a code base. You can sort of do that with per-cargo-package features but they are not convenient to use when you have deep dependency hierarchies. So some projects like the rust compiler opt to use RUSTFLAGS instead. I don't think it's something exclusive to the compiler, others might do it similarly. I think the blocker is for some mechanism for multi crate projects to share the list of available cfgs.

Edit: with your general argument over in rust-lang/rust#93628 (comment) that cfgs specified in RUSTFLAGS don't reach crates either, I think you are right that it's not useful. The rust compiler's bootstrap was running into the issue because it went out of its way to specify the bootstrap cfg for proc macros as well. So I think my second concern is resolved now.

@illicitonion
Copy link
Contributor

I recently released an update to a reasonably popular crate (thousands of reverse dependencies, 10M downloads) which had a significantly behaviour regression, and caused noticeable ecosystem breakage (including a spurious report of nightly being broken).

We had tests for the regression, but they were never getting run because they were hidden behind a feature, and there was a typo in the cfg(feature) attribute, meaning we weren't actually running those tests in CI.

I just tested out this feature for our example breakage, and it would have prevented us from releasing that broken code. I'd love to see it stabilise! The only modification I'd suggest is defaulting to deny rather than warn.

nyurik added a commit to nyurik/geozero that referenced this issue Apr 3, 2023
* Due to a typo in the cfg flag, tessellator was never built properly
  (we can even remove it as it is clearly not being used by anyone).
* This is a known issues in the rust compiler: rust-lang/cargo#10554
* I reverted Lyon to 0.16.2 and it worked fine.

Do we want to keep this and spend the effort to upgrade to the latest
Lyon crate, or is this a useless feature and should be removed?
nyurik added a commit to georust/geozero that referenced this issue Apr 3, 2023
* Due to a typo in the cfg flag, tessellator was never built properly
(we can even remove it as it is clearly not being used by anyone).
* This is a known issues in the rust compiler:
rust-lang/cargo#10554
* I reverted Lyon to 0.16.2 and it worked fine.
* I added a CI nightly `check` step to make sure this won't happen again

Do we want to keep this and spend the effort to upgrade to the latest
Lyon crate, or is this a useless feature and should be removed?
@ehuss ehuss added S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. labels Apr 25, 2023
@Urgau
Copy link
Contributor

Urgau commented Oct 23, 2023

Adjust -Zcheck-cfg for new rustc syntax and behavior - #12845 was merged and brings some major changes to the feature from Cargo side.

Unified options

All the options have been removed and -Zcheck-cfg no longer accepts any options (in the CLI or config.toml)

cargo build -Zcheck-cfg

Passing rustc-check-cfg: to cargo now requires the use of the new syntax.

New defaults

With the removal of all options passing -Zcheck-cfg now always enables features, well known names and well known values checking. This matches rustc defaults.

bors added a commit that referenced this issue Dec 1, 2023
Include declared list of features in fingerprint for `-Zcheck-cfg`

This PR include the declared list of features in fingerprint for `-Zcheck-cfg` (#10554)

`--check-cfg` verifies that `#[cfg()]` attributes are valid in Rust code, which includes `--cfg features foo` definitions that came from `[features]` tables in `Cargo.toml`.  For us to correctly re-check cfgs on feature changes,we need to include them in the fingerprint.

For example, if a user deletes an entry from `[features]`, they should then get warnings about any `#[cfg()]` attributes left in the code.  Without this change, that won't happen.  With this change, it now does.
@SteveLauC
Copy link

Do we miss the docsrs condition, which is used by the docs.rs site?

$ cat src/main.rs
$ /usr/bin/cat src/main.rs
#![cfg_attr(docsrs, feature(doc_cfg))]

fn main() {}

$ cargo +nightly check -Z unstable-options -Z check-cfg
warning: unexpected `cfg` condition name: `docsrs`
 --> src/main.rs:1:13
  |
1 | #![cfg_attr(docsrs, feature(doc_cfg))]
  |             ^^^^^^
  |
  = help: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
  = help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(docsrs)");` to the top of a `build.rs`
  = note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
  = note: `#[warn(unexpected_cfgs)]` on by default

warning: `rust` (bin "rust") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s

@Urgau
Copy link
Contributor

Urgau commented Jan 15, 2024

Do we miss the docsrs condition, which is used by the docs.rs site?

As far as I can tell, the docsrs config is a custom configuration and is not emitted by docs.rs, as can be seen in count-less Cargo.toml: rustdoc-args = ["--cfg", "docsrs"] [1] [2].

It is therefore not eligible to be a well known configuration and has to be (like any other custom config) be included "manually" either by:

  • adding println!("cargo:rustc-check-cfg=cfg(docsrs)"); to the top of a build.rs

  • using a Cargo feature

@SteveLauC
Copy link

As far as I can tell, the docsrs config is a custom configuration and is not emitted by docs.rs

Thanks for showing me this! TIL! Then we should whitelist it with println!("cargo:rustc-check-cfg=cfg(docsrs)");, it is so common that I thought it is something built-in:D

@epage
Copy link
Contributor

epage commented Jan 15, 2024

I feel like docsrs is a no-win situation

  • build.rs files come with a build-time / audit cost
  • However, there is no way to put an allow on a #[cfg]
  • Some of the docsrs usages have to appear at the top-level of a package (e.g. nightly features) and so you can't just allow in a more limited scope
  • Requiring libraries to do a blanket allow would just negate this feature
  • Switching to features is not a complete replacement. I try to use features where possible to make it easy to view my documentation locally (e.g. clap cookbook) but I use docsrs for nightly rust features, like auto-cfg, so I can locally run cargo check --all-features without it bombing out.

@Nemo157
Copy link
Member

Nemo157 commented Feb 11, 2024

I see there was previously a PR for declaratively specifying allowed cfg via the Cargo.toml #11631

I wonder if this idea could be resurrected for cases like fuzzing or loom (or nightly) where you don't necessarily have a build-script, but instead have some known cfg that will be injected by external tooling or manual RUSTFLAGS.

@epage

This comment was marked as resolved.

@epage
Copy link
Contributor

epage commented Feb 12, 2024

Using a feature would also be my preference. It particular using a feature integrates better with other crates that might use the same cfg.

nightly is a top-level concern, rather than a concern of the direct dependent.

I wonder if this idea could be resurrected for cases like fuzzing or loom (or nightly) where you don't necessarily have a build-script, but instead have some known cfg that will be injected by external tooling or manual RUSTFLAGS.

This would likely be picked back up via https://internals.rust-lang.org/t/pre-rfc-mutually-excusive-global-features/19618

@ijackson
Copy link
Contributor

I'm here because of the Call for Testing. I tried this with derive-adhoc which does some quite exciting things in its tests.

What I found:

  1. I had to add the println! to derive-adhoc-macros's build.rs. That build.rs only exists right now because it causes the OUT_DIR env var to be set by cargo. It would be nice to get rid of it. Adding more reasons for it is not great.
  2. I had to add the println! to a new build.rs in derive-adhoc. That's not great.
  3. When I did, I got the error below.
  4. I removed the rust-version from my Cargo.toml to get it to run and then I found that I would have to add a build.rs with a println! to several of my stunt test packages.

I stopped there. I think my conclusion is that a println in build.rs may be a bad approach.

error: the `cargo::` syntax for build script output instructions was added in Rust 1.77.0, but the minimum supported Rust version of `derive-adhoc v0.9.0 (/volatile/rustcargo/Rustup/Derive-Adhoc/derive-adhoc)` is 1.56.

@epage
Copy link
Contributor

epage commented Feb 15, 2024

I tried this with derive-adhoc which does some quite exciting things in its tests.

@ijackson could you go into more detail on how you are using --cfgs that you need to declare so many within your project?

When I did, I got the error below.

Also, you can use cargo: instead of removing or updating your MSRV.

I stopped there. I think my conclusion is that a println in build.rs may be a bad approach.

If we stabilize as-is, your only options will be

  • println
  • allow the warning
  • ignore the warning

What we need to be able to understand is

  • Was the feature working correctly
  • How representative your situation is to evaluate if the design is mature enough for stabilization

In the past, there was a proposal for a [cfg] table in Cargo.toml but this got rejected by the cargo team iirc. Generally, our hope is that https://internals.rust-lang.org/t/pre-rfc-mutually-excusive-global-features/19618 will be able to replace the last remaining cases of --cfg in people's workflows (except build.rs changing lib.rs) and that would do the check-cfgs for you like features.

@ijackson
Copy link
Contributor

Hi. Thanks for engaging.

@ijackson could you go into more detail on how you are using --cfgs that you need to declare so many within your project?

I'm not using --cfg. These are #[cfg(feature = ...)] but they aren't always declared in the Cargo.toml. This is because, for testing purposes I have multiple crates which consist of the same .rs files. For example, I have multiple versions of my main crate, and I use cargo features which exist only in the testing versions to modify the behaviour in a controlled way.

It is important that these weird testing features don't appear in the Cargo.toml of published crates.

When I did, I got the error below.

Also, you can use cargo: instead of removing or updating your MSRV.

Aha. (I'm surprised that this potential difficulty wasn't revealed when you tested your "call for testing", before publication.)

* `allow` the warning

I could use an allow but of course then I don't get the benefit of the new check. I guess I could reimplement something like the check ad-hoc with a perl script or something. Or just not have it checked; evidently I think that's tolerable since I haven't implemented an ad-hoc check myself so far :-).

That would be better than proliferating build scripts, with their effect on compile times etc.

What we need to be able to understand is

* Was the feature working correctly

None of the output suggested to me that it wasn't, apart from the complaint about MSRV.

* How representative your situation is to evaluate if the design is mature enough for stabilization

I don't know how common the techniques I have used are in the ecosystem. The key thing I'm doing is to use path = and/or symlinks to arrange that the same .rs files are used for what cargo thinks of are separate packages. The test packages have their own Cargo.tomls (which are semiautomatically maintained to be similar to the real packages).

I have found this an effective and convenient technique.

The "private version with different conditional compilations" is a bit like what #[cfg(test)] does - but my way allows testing of cross-crate behaviours. In derive-adhoc I use it to test cross-crate semver compatibility, and cross-crate import/export of derive-adhoc's template macros etc.

I think this is by far the most practical way of running these kinds of tests. The alternative would probably involve having each test run publish the current source code (perhaps with manglings) to a private testing registry, or something.

If anyone else is doing this kind of thing, you won't find the results on crates.io: after all one goal is to avoid advertising all the strange stuff. You could possibly find it by searching git repositories, but it's not clear to me what you'd meed to grep for.

@epage
Copy link
Contributor

epage commented Feb 21, 2024

I'm not using --cfg. These are #[cfg(feature = ...)] but they aren't always declared in the Cargo.toml. This is because, for testing purposes I have multiple crates which consist of the same .rs files. For example, I have multiple versions of my main crate, and I use cargo features which exist only in the testing versions to modify the behaviour in a controlled way.

It is important that these weird testing features don't appear in the Cargo.toml of published crates.

@ijackson that seems well off the beaten path. Why is it the features can't appear in Cargo.toml? Is it because you don't want people using them? docs.rs has the convention of hiding features that start with _ (#10794) and #10882 is about native support for that in cargo/docs.rs/crates.io

Why are these features needed for testing? What kind of tests are these?

@ijackson
Copy link
Contributor

@ijackson that seems well off the beaten path. Why is it the features can't appear in Cargo.toml? Is it because you don't want people using them? docs.rs has the convention of hiding features that start with _ (#10794) and #10882 is about native support for that in cargo/docs.rs/crates.io

Yes, that is why. As you observe, the support for such private features in the ecosystem (even cargo itself) isn't complete. There are open questions, like whether --all-features would still turn them on. Obviously I absolutely don't want --all-features to turn on my crazy testing stuff, so right now I can't use this convention.

Why are these features needed for testing? What kind of tests are these?

I don't think this tracking issue is quite the right place for a full explanation. Probably the best thing is for me to refer to some of the internal doc comments, eg: https://gitlab.torproject.org/Diziet/rust-derive-adhoc/-/blob/main/tests/pub-export/pub-b/pub-b.rs?ref_type=heads#L7

@Urgau
Copy link
Contributor

Urgau commented Feb 26, 2024

-Zcheck-cfg Stabilization Report 🎉

Summary

This enables rustc's checking of conditional compilation at compile time. Internally, cargo will be passing a new command line option --check-cfg to all rustc and rustdoc invocations.

By default, Cargo features will be checked along with well known cfg's (e.g. see rust-lang/rust#82450). Custom cfg's will be considered undefined and the user will need to specify them via cargo::rustc-check-cfg in their build scripts.

Since rust#117522, this only affects #[cfg]s in code and does not affect setting custom --cfg via RUSTFLAGS .

Example

cargo check
[package]
name = "y"
version = "0.1.0"
edition = "2021"

[features]
lasers = []
zapping = []
#[cfg(feature = "lasers")]  // This condition is expected, as "lasers" is an expected value of `feature`
fn shoot_lasers() {}

#[cfg(feature = "monkeys")] // This condition is UNEXPECTED, as "monkeys" is NOT an expected value of
                            // `feature`
fn write_shakespeare() {}

#[cfg(windosw)]             // This condition is UNEXPECTED, it's supposed to be `windows`
fn win() {}
warning: unexpected `cfg` condition value: `monkeys`
 --> src/lib.rs:4:7
  |
4 | #[cfg(feature = "monkeys")] // This condition is UNEXPECTED, as "monkeys" is NOT an expected valu...
  |       ^^^^^^^^^^^^^^^^^^^
  |
  = note: expected values for `feature` are: `lasers`, `zapping`
  = help: consider adding `monkeys` as a feature in `Cargo.toml`
  = note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
  = note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition name: `windosw`
 --> src/lib.rs:8:7
  |
8 | #[cfg(windosw)]             // This condition is UNEXPECTED, it's supposed to be `windows`
  |       ^^^^^^^ help: there is a config with a similar name: `windows`
  |
  = help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(windosw)");` to the top of a `build.rs`
  = note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration

warning: `foo` (lib) generated 2 warnings

Well known cfgs

When the feature is enabled, Cargo inherits the well-known names and values from rustc. These well known cfgs make the feature near-zero impact for the vast majority of users, as they don't need to mark them as expected.

Cargo also adds a few of it's own:

  1. feature with the list of features
  2. docsrs (as it has a closer relationship with the crate ecosystem and by extension Cargo than rustc).

Performance

The feature has very little performance impact, zero for small crates and a few milliseconds max for huge crates (>99000 cfgs, see below).

Was done using hyperfine on an Intel Core i7-7700HQ (4 cores, 3.8GHz max).

They represent both extreme, helloworld (0 feature and 1 cfg) and windows-rs (~700 features and >99000 cfgs in the code).

windows crate

$ hyperfine --prepare "cargo clean" "cargo +nightly check" "cargo +nightly check -Zcheck-cfg"
Benchmark 1: cargo +nightly check
  Time (mean ± σ):     805.4 ms ±  12.5 ms    [User: 675.2 ms, System: 150.8 ms]
  Range (min … max):   792.1 ms … 824.6 ms    10 runs

Benchmark 2: cargo +nightly check -Zcheck-cfg
  Time (mean ± σ):     822.5 ms ±  11.9 ms    [User: 691.1 ms, System: 150.7 ms]
  Range (min … max):   803.6 ms … 836.6 ms    10 runs

Summary
  cargo +nightly check ran
    1.02 ± 0.02 times faster than cargo +nightly check -Zcheck-cfg

Or around +17.1ms for 4 (local) crates, with the being one being at ~700 features and 99102 total #[cfg]s in the code!

(Also note that there was 10/15 warnings and they were not suppressed so the linting/display machinery was printing them)

helloworld (lib)

$ hyperfine --prepare "cargo clean" "cargo +nightly check" "cargo +nightly check -Zcheck-cfg"
Benchmark 1: cargo +nightly check
  Time (mean ± σ):      67.5 ms ±   1.1 ms    [User: 30.3 ms, System: 34.9 ms]
  Range (min … max):    65.9 ms …  69.2 ms    20 runs

Benchmark 2: cargo +nightly check -Zcheck-cfg
  Time (mean ± σ):      67.4 ms ±   0.9 ms    [User: 31.0 ms, System: 34.0 ms]
  Range (min … max):    65.4 ms …  69.4 ms    21 runs

Summary
  cargo +nightly check -Zcheck-cfg ran
    1.00 ± 0.02 times faster than cargo +nightly check

No visible impact.

Enable-by-default

Per the tracking issue, there is one last but important question to address:

Should these flags be enabled by default? What's the unintrusive interface to provide for users to opt-out the checking?

I propose that the behavior be enabled by default without a way to opt out, otherwise users will probably not see their mistakes in time or not know about the flag at all.

Impact

A crater run was done in rust#120701, the full summary can be found there. The sort version is:

After manually checking 3 categories (most unexpected features, target_*, and general) I wasn't able to find any false positive1; in total 9649 actions would need to be taken across 5959 projects (1.4% of all the projects tested), by either: fixing the typo, removing the staled condition, marking as expected the custom cfg, adding the feature to the features table...

A Call for testing was also performed and the responses were generaly positive, there were questions about the default set well know names/values and some anti pattern #[path] file sharing, but those are ortogonal to this stabilization.

Mitigations options

However to mitigate it's impact we probably want to annonce the feature ahead of time and provide ahead of time a "migration guide" for users of custom configs.

None

We can consider that the impact is minimal enough and that positives of the feature are strong enough to not do anything more.

Edition bound

We have a edition (2024) that is approaching, we could bound the feature to being enabled on being enabled only for edition>=2024.

on Cargo side

We would only pass --check-cfg on newer editions.

on rustc side

We (Cargo) would always pass --check-cfg but the lint would be allow by default for edition<2024 and warn-by-default for edition>=2024.

Documentation and Testing

The feature is currently documented in the check-cfg section of the Cargo unstable features chapter. Stabilizing the feature would mainly involve documentating cargo::rustc-check-cfg as stable and providing users with a "migration page".

The feature is being extensively tested under tests/testsuite/check_cfg.rs (~20 tests). To give a some highlights:

Real World Usage

Unresolved questions

  • How to deal with the impact of enabling the feature? See above for run down of the options.
  • Should the build script directive warning be appeased ahead of the stabilization ?

Footnotes

  1. by "false positive" I mean: missing well known names/values

@epage
Copy link
Contributor

epage commented Feb 26, 2024

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Feb 26, 2024

Team member @epage has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge final-comment-period FCP — a period for last comments before action is taken and removed proposed-final-comment-period An FCP proposal has started, but not yet signed off. labels Feb 26, 2024
@rfcbot
Copy link
Collaborator

rfcbot commented Feb 28, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot
Copy link
Collaborator

rfcbot commented Mar 9, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@bors bors closed this as completed in 7ebc065 May 3, 2024
mqudsi added a commit to fish-shell/fish-shell that referenced this issue May 9, 2024
rustc 1.80 now complains about features not declared in Cargo.toml and cfg
keys/values not declared by build.rs to protect against typos or misuse (you
think you're using the right condition but you're not). See
rust-lang/cargo#10554 and rust-lang/rust#82450.

(We're not actually using TSAN under CI at this time, but I do want to re-enable
it at some point — especially if we get multithreaded execution going — using
the rust-native TSAN configuration.)

I'll be updating the `rsconf` crate and patching `build.rs` accordingly to also
handle the warnings about unknown cfg values, but tsan is a feature and not a
cfg and these can be dealt with in `Cargo.toml` directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for something unstable. disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. T-cargo Team: Cargo to-announce
Projects
Status: Unstable, baking
Archived in project
Roadmap
  
Unstable, baking
Development

Successfully merging a pull request may close this issue.

10 participants