search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.56k stars 12.47k forks source link

Tracking Issue for RFC 3013: Checking conditional compilation at compile time #82450

Closed ehuss closed 4 months ago

ehuss commented 3 years ago

This is a tracking issue for the RFC 3013: Checking conditional compilation at compile time (rust-lang/rfcs#3013).

Issues: https://github.com/rust-lang/rust/labels/F-check-cfg Documentation: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html

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.

Steps

Unresolved Questions

Implementation history

Urgau commented 2 years ago

The implementation of this RFC has been merged in rustc as unstable and is already in nightly for experimentation. :tada:

The next the steps for the rustc side will be to improve it with suggestions, auto-fix in obvious cases and hopefully add well known values (non-builtin target is a problem), the rfc doesn't mention it directly but this seems pretty obvious. Some of this stuff is already tackle by https://github.com/rust-lang/rust/pull/94175.

We almost certainly want rustdoc to also have it (because rustc doesn't know about doc tests and rustdoc also have --cfg so it seems good to be consistent), so I've opened https://github.com/rust-lang/rust/pull/94154 to tackle it.

And last but not least the cargo integration, the RFC intentionally left the control part out of the RFC, but I have some ideas about how it could be done. But as RFC said it seems uncontroversial for Cargo to enable checking for feature = "..." values, so I'm currently tackling this part. ~No PR for this yet, but I'm working on it.~

EDIT: Cargo PR https://github.com/rust-lang/cargo/pull/10408, merged ~but not yet in nightly~ see https://github.com/rust-lang/rust/issues/82450#issuecomment-1052085034

jhpratt commented 2 years ago

Wow, that's awesome! I didn't even know this was being worked on actively.

As someone who would like love to use this in CI, how feasible is it to try this on nightly, even if "just" for features? I ask because it sounds like cargo integration isn't a thing yet.

Urgau commented 2 years ago

Wow, that's awesome! I didn't even know this was being worked on actively.

:heart:

As someone who would like love to use this in CI, how feasible is it to try this on nightly, even if "just" for features? I ask because it sounds like cargo integration isn't a thing yet.

It would just be a matter of passing something like this -Z unstable-options --check-cfg='values(feature, "f_a", "f_b", "f_n")' where f_a, f_b, ... f_n are your features name to rustc with cargo you can use the RUSTFLAGS env variable do to it. This manual process is only temporary until cargo support is added.

RUSTFLAGS='-Z unstable-options --check-cfg=values(feature,"f_a","f_b","f_n")' cargo build
jhpratt commented 2 years ago

That's not bad at all! I'll definitely add in a CI check.

Urgau commented 2 years ago

Good news! rustdoc (https://github.com/rust-lang/rust/pull/94154) and cargo (only features, https://github.com/rust-lang/cargo/pull/10408) support have been merged. :fireworks: As well a some improvements to the lint (https://github.com/rust-lang/rust/pull/94175).

You can now simply use cargo build -Z check-cfg-fetaures to check values of every config feature. Works with: check, build and test.

$ cargo +nightly check -Z check-cfg-features
    Checking hoho v0.1.0 (/tmp/hoho)
warning: unexpected `cfg` condition value
 --> src/main.rs:1:7
  |
1 | #[cfg(feature = "tokiO")]
  |       ^^^^^^^^^^-------
  |                 |
  |                 help: did you mean (notice the capitalization): `"tokio"`
  |
  = note: `#[warn(unexpected_cfgs)]` on by default
  = note: expected values for `feature` are: default, serde, tokio

warning: `hoho` (bin "hoho") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.54s
c410-f3r commented 2 years ago

Thank you for all the hard work, @Urgau

petrochenkov commented 2 years ago

Enabling --check-cfg=values() currently incurs some compilation time hit (especially on small programs) because it will load target specifications for all targets (100-200 of them at the moment), while without --check-cfg=values() targets are only loaded on demand (so only 1-2 target specs are typically loaded).

In theory, majority of targets can be loaded at compiler's compile time instead (and put into a static). In that case only a few Apple targets will be loaded dynamically because they need to read environment variables from the host. The main blocker for this is supporting String::from("foo") at compile time (i.e. const allocations).

petrochenkov commented 2 years ago

Enabling --check-cfg=values() currently incurs some compilation time hit (especially on small programs)

Someone also needs to prepare a perf run to measure the exact slowdown.

lcnr commented 2 years ago

The main blocker for this is supporting String::from("foo") at compile time (i.e. const allocations).

Alternatively one could use some Cow or whatever for targets instead of String.

Urgau commented 2 years ago

Small update.

Work was done to reduce the cost associated with well known values checking in PR https://github.com/rust-lang/rust/pull/95202. It resulted in noticeable win, coming from +1.5% to +1%. More work could be done in the future with improvements in const functions.

The cargo integration has also evolved quite a bit with the introduction of a single flag: -Zcheck-cfg that take one or multiple values (https://github.com/rust-lang/cargo/pull/10566):

cargo check -Z check-cfg=features,names,values

Another big addition on the cargo side is the introduction of the build output rustc-check-cfg in build script (https://github.com/rust-lang/cargo/pull/10539):

// build.rs
println!("cargo:rustc-check-cfg=names(foo, bar)");
cargo check -Z check-cfg=output

More information can be found on cargo reference: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg

Kampfkarren commented 2 years ago

Would've loved to have this the other day. I wrote out #[cfg(feature = "debug_assertions")] in a ton of places instead of #[cfg(debug_assertions)].

Wyvern commented 2 years ago

Hi guys, following this thread put -Zcheck-cfg=features,names,values,output as command args works, but is there a way to put it in config.toml [unstable] section for now? Tried several times, still got parse errors...

Urgau commented 2 years ago

Hi guys, following this thread put -Zcheck-cfg=features,names,values,output as command args works, but is there a way to put it in config.toml [unstable] section for now? Tried several times, still got parse errors...

@Wyvern Sorry for the inconvenience, there is currently no way to set it because of a mistake from my part when changing a not so internal struct. I've posted a fix the cargo side https://github.com/rust-lang/cargo/pull/10799 (it should take at least a few days before being available in nightly).

Urgau commented 10 months ago

As part of my effort to improve check-cfg I opened several months ago MCP636 - Simplify and improve explicitness of the check-cfg syntax, which was accepted. The following represent the user-facing changes.

rustc changes

Add new simpler and more explicit syntax for check-cfg - #111072

New cfg form

It introduces the new cfg form and deprecate the previous two (names and values):

rustc --check-cfg 'cfg(name1, ..., nameN, values("value1", "value2", ... "valueN"))'

Default built-in names and values

It also changes the default for the built-in names and values checking.

cargo changes

Adjust -Zcheck-cfg for new rustc syntax and behavior - #12845

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

This means that passing -Zcheck-cfg now always enables features, well known names and well known values checking.

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

If you have any remarks/issues feel free to open an issue about it or contact me on rust-lang Zulip instance.

jplatte commented 10 months ago

There is no way to mark existing well-known attributes as unknown, is there? Specifically, I think it would be nice if there was a way for rustc to raise a warning when it encounters cfg(test) (which cargo could then make use of for crates that use lib.test = false).

epage commented 9 months ago

We've been discussing --check-cfg checking --cfg (rust-lang/rust#100574) on zulip

So to step back, let's put this in scope of the use cases where --check-cfg helps:

Usually a mixture of these will exist within a workspace and its non-local dependencies

For me, this seems like it offers a lot of benefits and would love to see this stabilized as warning by default for cargo projects.

For Case 5, a user might forget the check-cfg directive and things just work because their #[cfg] is behind another #[cfg]. When the root #[cfg] is active, they can get an unexpected cfg lint for the valid #[cfg].

This also helps with Case 6 by reporting warnings when the user mistypes a --cfg

Case 3 and 4 are the problematic ones. For the packages that consume the cfg, they can have a build.rs directive declare it but that won't help the compilation of build.rs and rustc will emit a warning about an unexpected cfg on the command-line. Rustc will also emit a warning for every other package's build-targets (and their build script) in the build graph. Cargo passes --cap-lints. This means that the above false positives will only show up per workspace member and not per dependency. It also means means that --cfg being checked by --check-cfg won't catch typos in tokio in Case 3. Even if it does catch it in Case 4, it would be lost in the noise.

If we stabilized as-is, workarounds by users include

RUSTFLAGS=--cfg is a more advanced use case (which is why we don't support well known cfgs to be defined in Cargo.toml). The cargo team would love to find higher levels of abstraction to replace RUSTFLAGS (see rust-lang/cargo#12739) and globals is one possible replacement for users interacting with --cfg as much.

That said, RUSTFLAGS and --cfg is where we are at now and we have no idea if or when abstractions will come along. And while its generally an advanced case, a tepid user dipping their toes into this thing recommended in a document somewhere shouldn't get a hostile experience that makes them lose what confidence they had in working with Rust.

With all of that said, cargo is not the only user of rustc and we need to consider what the correct mechanism / "API" rustc should provide to build systems (or people directly invoking it).

So where do we go from here?

Some options

ehuss commented 9 months ago

For the packages that consume the cfg, they can have a build.rs directive declare it and no warning will happen.

Passing cargo:rustc-check-cfg=cfg(…) in a build script doesn't silence the warning because the --cfg flag is passed when building the build script itself, and thus will issue a warning. That's partially why #11631 was proposing to define the valid cfg's in Cargo.toml.

Having users pass additional flags in RUSTFLAGS (like -A or --check-cfg) doesn't sound too awful, but it is definitely clunky, and will introduce a noisy experience for some users. I wish we had better options for the use cases where users are doing that. If we move forward with this as-is, I think we should at least do some kind of education for users to guide them on how to fix it.

epage commented 9 months ago

Passing cargo:rustc-check-cfg=cfg(…) in a build script doesn't silence the warning because the --cfg flag is passed when building the build script itself, and thus will issue a warning. That's partially why https://github.com/rust-lang/rust/issues/11631 was proposing to define the valid cfg's in Cargo.toml.

Ugh, overlooked that. I assume these are unique enough that there is no de-duplicating (if we have any) and so the user will see a warning per package and build script.

Having users pass additional flags in RUSTFLAGS (like -A or --check-cfg) doesn't sound too awful, but it is definitely clunky, and will introduce a noisy experience for some users. I wish we had better options for the use cases where users are doing that. If we move forward with this as-is, I think we should at least do some kind of education for users to guide them on how to fix it.

For me, the biggest issue is that this feature is meant to help people discover when they type something in and the very first time they try to use --cfg, they will be inundated with messages saying they did it wrong. Education has a very limited reach in helping people ignore this rather than assume they did something wrong and then they go through a long investigation to figure out why.

Once they know it (or were already "in the know"), then that is a functional, albeit clunky, workaround.

ehuss commented 9 months ago

I assume these are unique enough that there is no de-duplicating (if we have any) and so the user will see a warning per package and build script.

It will get de-duped, but there are summaries for each package that look like warning: `foo` (build script) generated 1 warning (1 duplicate).

Also, in the scenario where you don't pass --check-cfg anywhere, then you'll get a warning for every target in a package (build-script, lib, bins, tests, examples, etc.). So the "duplicate" warning message could potentially show up a large number of times. But on the positive side, it is only one line (for each target).

jplatte commented 9 months ago

There's also the case of people using #![cfg_attr(docsrs, feature(doc_auto_cfg)] and such. Luckily, this can be solved with a build script like I did here since that cfg is only ever actually passed via RUSTDOCFLAGS (and we don't use --check-cfg with it, so only need the build script for workspace members that have a docsrs attribute).

epage commented 9 months ago

we don't use --check-cfg with it, so only need the build script for workspace members that have a docsrs attribute

My hope / expectation is that we'll make --Zcheck-cfg the exclusive behavior of cargo when stabilized and I assume that would apply to cargo doc as well.

However, warnings during docs.rs's call to cargo doc so checking --cfg will likely not be a problem for this use case.

Nemo157 commented 9 months ago

However, warnings during docs.rs's call to cargo doc so checking --cfg will likely not be a problem for this use case.

I do hope to one day surface warnings from docs.rs' builds to crate maintainers.

Urgau commented 7 months ago

Status update, as of 2024-01-18.

Two new PRs have been merged, #119473 and #119930. They come from a feedback from Cargo (thanks @epage), where we realised that empty values() were confusing and not intuitive.

These two changes have allowed Cargo to go back to always using values(...) (even if there are zero declared features), which makes the Cargo --check-cfg invocation more in line with reality and allows for better diagnostics. See https://github.com/rust-lang/cargo/pull/13316 for further details.

Urgau commented 6 months ago

Status update, as of 2024-02-10:

  1. A Crater run has been done in #120701, the full analysis report can be found here. The here is (very) condensed summary:

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

    [^false_positives]: by "false positive" I mean: missing well known names/values

  2. A Call for Testing has been issued in the RFC PR, it should appear in next week (2024-10-15) This Week in Rust.
ojeda commented 6 months ago

Thanks for this feature!

I gave it a try in the kernel: it was easy to set up and seemed to work as expected, with nice errors for small typos, e.g.:

error: unexpected `cfg` condition name: `CONFIG_KVNIT`
  --> rust/kernel/lib.rs:38:7
   |
38 | #[cfg(CONFIG_KVNIT)]
   |       ^^^^^^^^^^^^ help: there is a config with a similar name: `CONFIG_KUNIT`

Here are a few notes I noticed in a first pass:

epage commented 6 months ago

@ojeda any performance concerns with checking 20k cfg's? We did some basic testing on the windows package because that has "a lot" of features but 20k is in a category of its own.

Can those 20k cfg's be scoped down to relevant subsystems? I can understand the kernel as a whole having a lot but I would assume most of those are meant for specific subsystems and scoping could improve the messages and improve the checking for developers.

Urgau commented 6 months ago

@ojeda Thanks you for testing the future.

  • The kernel has ~20k (yes, kilo!) configuration options. For a quick test on this feature, I transformed the ~3k --cfg flags we pass in a small config (i.e. the enabled features) into --check-cfgs, but for proper checking we will need to pass all the 20k symbols. So this is another data point on having many config options, like the one someone else posted.

:-) that's a lot. Did you use one --check-cfg argument per config or did you use the shorthand^1 (one --check-cfg for all configs) ?

The shorthand version might at your scale make things a bit faster to decode.

Also did you need to use a @path argument?

  • The error message for an unknown condition name dumps all the known names in some cases, which is a bit too much (pages of the terminal) when you have thousands of those... :)

Yeah, that's not fine. I will send a fix for it, probably similar to our very long types where we write the full type to disk.

  • This help note is not really appropriate for calls to rustc that do not come from Cargo: 
         = help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(CONFIG_KUNI)");` to the top of a `build.rs`

I thought of you (non Cargo users) when writing this diagnostic and like any other Cargo-specific diagnostic, it is conditional on the CARGO env being set. Could you check that it was or wasn't set?

  • Checking seems to be enabled just by passing -Zunstable-options -- is that expected? i.e. without passing any --check-cfg flag.

I'm not able to reproduce this.

$ echo "#[cfg(uniz)] fn test() {}" > a.rs
$ rustc +nightly --crate-type=lib -Zunstable-options a.rs # does NOT lint
$ rustc +nightly --crate-type=lib -Zunstable-options --check-cfg='cfg()' a.rs # does lint
ojeda commented 6 months ago

@ojeda any performance concerns with checking 20k cfg's? We did some basic testing on the windows package because that has "a lot" of features but 20k is in a category of its own.

I only tested the case with ~4k --check-cfgs (one for each option), but to give a rough idea, in a noisy VM, I get 585ms vs. 537ms for our kernel crate, i.e. about ~48ms, ~9%.

I gave a quick try to the grouping suggestion and I got 546ms, i.e. about ~9ms, ~2%.

So this could be significant (seconds) when we start having many Rust kernel modules for big configs, but in the worst case I guess we could skip it for normal builds and only check it in particular builds.

Can those 20k cfg's be scoped down to relevant subsystems? I can understand the kernel as a whole having a lot but I would assume most of those are meant for specific subsystems and scoping could improve the messages and improve the checking for developers.

It would require manual work maintaining the lists (and kernel developers are used to having all the configuration available), so maintaining custom lists would need to be useful for something else to be worth the tradeoff.

Did you use one --check-cfg argument per config or did you use the shorthand1 (one --check-cfg for all configs) ?

In the quick test I did, one per config, with many looking like this:

--check-cfg=cfg(CONFIG_SATA_SIS, values(none(), "y", "n", "m"))

But we could definitely do a single --check-cfg for e.g. all boolean options, then one for all the tristates, etc. I tried that -- please see above.

Also did you need to use a @path argument?

Yeah (we are already using @path for all the --cfgs we have).

Yeah, that's not fine. I will send a fix for it, probably similar to our very long types where we write the full type to disk.

Thanks! However, I wouldn't write the list to disk -- I don't think it is useful to see the list, and we would need to ignore those files in .gitignore and so on.

I thought of you (non Cargo users) when writing this diagnostic and like any other Cargo-specific diagnostic, it is conditional on the CARGO env being set. Could you check that it was or wasn't set?

Ah, I see; yeah, we use the variable to point to the Cargo binary (like we do for other compilers/tools).

Should that behavior be documented in src/doc/rustc, similar to Cargo that documents its environment variables? If it is not documented already, perhaps it could be changed to another variable. Otherwise, I guess we could unset it for rustc calls, but it is a bit unfortunate.

I'm not able to reproduce this.

My mistake -- it happened with core, for which I thought I was not passing the @path, but I do, so please ignore that.

Urgau commented 6 months ago

Thanks! However, I wouldn't write the list to disk -- I don't think it is useful to see the list, and we would need to ignore those files in .gitignore and so on.

@ojeda I'm a bit surprised by this; while I understand that the kernel may not need this because of documentation and other things, I'm not sure this can be generalized. Other build systems may not expose the full list easily, debugging is also an area where it is very useful to see the full list.

Do Rust-for-Linux pass -Zwrite-long-types-to-disk=no to rustc ? What would you think about a similar flag but for printing the full list ?

Ah, I see; yeah, we use the variable to point to the Cargo binary (like we do for other compilers/tools).

Should that behavior be documented in src/doc/rustc, similar to Cargo that documents its environment variables? If it is not documented already, perhaps it could be changed to another variable. Otherwise, I guess we could unset it for rustc calls, but it is a bit unfortunate.

I've looked at this, and I see at least 5 different usage of "does the CARGO env exist" to imply that rustc was called from Cargo. There are also usage of CARGO_CRATE_NAME and CARGO_PKG_NAME for roughly the same thing.

I don't think it is documented anywhere, since I think no one thought of users setting CARGO but not using it. We may want to switch to another Cargo env; and just to be sure, you're not setting any other Cargo specific env?

epage commented 6 months ago

I'm a bit surprised by this; while I understand that the kernel may not need this because of documentation and other things, I'm not sure this can be generalized. Other build systems may not expose the full list easily, debugging is also an area where it is very useful to see the full list.

Maybe its just me but I feel like if the list of suggestions is too long, its just not worth doing anything with. I don't see it being likely someone is going to open a file with 29k options to find the exact one they wanted.

ojeda commented 6 months ago

@ojeda I'm a bit surprised by this; while I understand that the kernel may not need this because of documentation and other things, I'm not sure this can be generalized. Other build systems may not expose the full list easily,

Before this option existed, developers already needed a way to know the list somehow, i.e. they needed it to use the cfgs to begin with. For instance, in the context of the kernel, one may check the Kconfig files (which contain way more information than just the names, like descriptions, dependencies, they are better organized, etc.).

In the typo case, the suggestion is nice and I can see it saving time, but in the full list case, if there was no close match to something I wrote, then I should probably go and check what is going on, rather than try to pick one from a long list (even a list of 50 elements would already be way too much, I would say).

So I can see the full list perhaps being useful in simple cases where there are not many to pick from, and where you already know more or less what they do, so you can copy-paste it from there.

debugging is also an area where it is very useful to see the full list.

I am not sure what you mean by this.

Do Rust-for-Linux pass -Zwrite-long-types-to-disk=no to rustc ? What would you think about a similar flag but for printing the full list ?

We don't use it, but it seems a bit of a different case to me: seeing the full type may be needed in some cases, because it is information that may not be anywhere else.

But seeing the full list of config options (in the kernel case at least) does not give you any new information, and in fact it gives you less than the usual ways kernel developers would use. So I don't see why a kernel developer would want that file.

So if the flag would allow to not just select terminal vs. file but to disable it, it would be definitely nice for us. Having said that, I still wonder if someone would find useful a very big file for these purposes (outside the kernel usage), i.e. it may be best to simply cut the list short instead, or give the closest 20 options, plus a count of not-shown options, or something like that (i.e. rather than give options with a flag).

I've looked at this, and I see at least 5 different usage of "does the CARGO env exist" to imply that rustc was called from Cargo. There are also usage of CARGO_CRATE_NAME and CARGO_PKG_NAME for roughly the same thing.

I don't think it is documented anywhere, since I think no one thought of users setting CARGO but not using it. We may want to switch to another Cargo env; and just to be sure, you're not setting any other Cargo specific env?

If you mean the ones that Cargo documents at https://doc.rust-lang.org/cargo/reference/environment-variables.html, we set things like RUSTC, RUSTDOC, RUSTFMT, RUSTFLAGS... But we do not do it for Cargo; it is just that the kernel Makefile sets early on variables for the programs, e.g. CC, PYTHON3 etc., and since we currently use Cargo somewhere for unrelated reasons, we had CARGO too.

That is why we collided here -- perhaps it would be indeed a good idea to make rustc and Cargo communicate this fact in another way that is less prone to such collisions, especially if it is not going to be documented or if it can influence more important behavior.

ojeda commented 6 months ago

Maybe its just me but I feel like if the list of suggestions is too long, its just not worth doing anything with. I don't see it being likely someone is going to open a file with 29k options to find the exact one they wanted.

Yeah, exactly.

Urgau commented 6 months ago

Maybe its just me but I feel like if the list of suggestions is too long, its just not worth doing anything with. I don't see it being likely someone is going to open a file with 29k options to find the exact one they wanted.

Yeah, exactly.

Point taken.

[..] perhaps it would be indeed a good idea to make rustc and Cargo communicate this fact in another way that is less prone to such collisions, especially if it is not going to be documented or if it can influence more important behavior.

So using something very Cargo specific like CARGO_PKG_NAME would be fine for you? @epage any suggestion for a Cargo flag passed to rustc that has a low chance of collision with other build system and that is always set by Cargo ?

ojeda commented 6 months ago

Yeah, that would be fine (or at least I don't see why it wouldn't -- is there a particular concern I should be aware of?).

But at least for this it is not a big deal, we could live even with the current situation (but please let me know if CARGO has other consequences that may be more important to know about -- that is why I suggest documenting it).

Having said that, if you are considering improving this, perhaps it may be a good opportunity to pick a new name for all the suggestions, e.g. RUSTC_SUGGEST_CARGO_DIAGNOSTICS (and document it in the rustc side), or maybe better a rustc flag --suggest-cargo-diagnostics.

epage commented 6 months ago

I have no preference on an existing environment variable to detect a cargo environment.

Urgau commented 6 months ago

--check-cfg Stabilization Report :tada:

Summary

This adds a new top-level command line option --check-cfg (to rustc and rustdoc) in order to enable checking conditional compilation at compile time.

rustc will check every #[cfg(name = "value")], #[cfg_attr(name = "value")], #[link(name = "a", cfg(name = "value"))] attributes and cfg!(name = "value") macro calls. --cfg arguments are not checked[^arg].

[^arg]: For a big part of the existance of the feature, --cfg CLI args were also checked, but given that it interacted badly Cargo RUSTFLAGS, it was removed and left as an future posibility in the documentation.

The syntax looks (roughly) like this:

rustc --check-cfg 'cfg(name, values("value1", "value2", ... "valueN"))'

From the [unstable book documentation](https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html#the-cfg-form): > To enable checking of values, but to provide an *none*/empty set of expected values (ie. expect `#[cfg(name)]`), use these forms: > > ```bash > rustc --check-cfg 'cfg(name)' > rustc --check-cfg 'cfg(name, values(none()))' > ``` > > To enable checking of name but not values, use one of these forms: > > - No expected values (_will lint on every value_): > ```bash > rustc --check-cfg 'cfg(name, values())' > ``` > > - Unknown expected values (_will never lint_): > ```bash > rustc --check-cfg 'cfg(name, values(any()))' > ``` > > To avoid repeating the same set of values, use this form: > > ```bash > rustc --check-cfg 'cfg(name1, ..., nameN, values("value1", "value2", ..., "valueN"))' > ``` > > The `--check-cfg cfg(...)` option can be repeated, both for the same condition name and for different names. If it is repeated for the same condition name, then the sets of values for that condition are merged together (precedence is given to `values(any())`). > Like with `values(any())`, well known names checking can be disabled by passing `cfg(any())` as argument to `--check-cfg`.

Example

rustc --check-cfg 'cfg(is_embedded, has_feathers)' \
      --check-cfg 'cfg(feature, values("zapping", "lasers"))' foo.rs
#[cfg(is_embedded)]         // This condition is expected, as 'is_embedded' was provided in --check-cfg
fn do_embedded() {}         // and doesn't take any value

#[cfg(has_feathers)]        // This condition is expected, as 'has_feathers' was provided in --check-cfg
fn do_features() {}         // and doesn't take any value

#[cfg(has_mumble_frotz)]    // This condition is UNEXPECTED, as 'has_mumble_frotz' was NEVER provided
                            // in any --check-cfg arguments
fn do_mumble_frotz() {}

#[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() {}
warning: unexpected `cfg` condition name: `has_mumble_frotz`
 --> foo.rs:7:7
  |
7 | #[cfg(has_mumble_frotz)]
  |       ^^^^^^^^^^^^^^^^
  |
  = help: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `has_feathers`, `is_embedded`, `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: to expect this configuration use `--check-cfg=cfg(has_mumble_frotz)`
  = note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
  = note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition value: `monkeys`
  --> foo.rs:14:7
   |
14 | #[cfg(feature = "monkeys")]
   |       ^^^^^^^^^^^^^^^^^^^
   |
   = note: expected values for `feature` are: `lasers`, `zapping`
   = help: to expect this configuration use `--check-cfg=cfg(feature, values("monkeys"))`
   = note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration

warning: 2 warnings emitted

Well known names and values

rustc has a internal list of well known names and their corresponding values. Those well known names and values follows the same stability as what they refer to.

This list exist so that users don't have to manually define them.

The target_* are automatically derived from the builtin targets.

As of `2024-02-15T`, the list of known names is as follows: - `clippy` - `debug_assertions` - `doc` - `doctest` - `docsrs` _(set by Cargo)_ - `feature` _(set by Cargo)_ - `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`

Cargo integration

Since ~1.61 nightly Cargo there has been some form of integration of --check-cfg in Cargo with the -Zcheck-cfg.

The latest and current one is -Zcheck-cfg which:

Enables checking conditional compilation at compile time with rustc --check-cfg. It enables feature, well known names, well known values and cargo::rustc-check-cfg (in build scripts).

A stabilization report for Cargo will be done and would propose to make the behavior of -Zcheck-cfg the default, see rust-lang/cargo#10554.

Documentation and Testing

The feature is described in the unstable book under it's own section.

The feature is being extensively tested under tests/ui/check-cfg (27 test files). To give a some highlights:

Real World Usage

[^rfl]: from the Call for testing

Unresolved questions

lnicola commented 6 months ago

Well known names and values

I've seen #[cfg(FALSE)] recommended and used as a way to disable code. I guess it's not needed if #[cfg(any())] works, but it's not really obvious.

c410-f3r commented 6 months ago

Regardless of the acceptance of the respective teams, I would like to congratulate @Urgau for assuming the responsible to implement and stabilize this important feature. It is not rare to see issues stalled for years so thank you very much!

wesleywiser commented 6 months ago

@rfcbot fcp merge

rfcbot commented 6 months ago

Team member @wesleywiser 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 commented 5 months ago

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

rfcbot commented 4 months ago

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.