From 2f9961c4858075404f49db9ded19d4ff9de02491 Mon Sep 17 00:00:00 2001 From: Arlie Davis Date: Tue, 3 Nov 2020 15:07:02 -0800 Subject: [PATCH 01/12] check-cfg RFC draft --- text/0000-conditional-compilation-checking.md | 405 ++++++++++++++++++ 1 file changed, 405 insertions(+) create mode 100644 text/0000-conditional-compilation-checking.md diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md new file mode 100644 index 00000000000..2dea1e8fcf0 --- /dev/null +++ b/text/0000-conditional-compilation-checking.md @@ -0,0 +1,405 @@ +# Checking conditional compilation at compile time + +# Summary + +Rust supports conditional compilation, analogous to `#ifdef` in C / C++ / C#. Experience has shown +that managing conditional compilation is a significant burden for large-scale development. One of +the risks is that a condition may contain misspelled identifiers, or may use identifiers that are +obsolete or have been removed from a product. For example: + +```rust +#[cfg(feature = "widnows")] +fn do_windows_thing() { /* ... */ } +``` + +The developer intended to test for the feature named `windows`. This could easily have been detected +by `rustc` if it had known the set of all valid `feature` flags, not only the ones currently +enabled. Cargo already has this information, but it does not communicate it to `rustc`. This RFC +proposes a means for `rustc` to check this information. + +# Motivation + +* Stronger assurance that large code bases are correct. +* Protect against typos, bad merges, etc. +* Detect dead code, typically caused by feature flags that have been removed from a crate's + manifest, but which still have `#[cfg(...)]` attributes that mention those features. + +# Guide-level explanation + +## Background + +Rust programs can use conditional compilation in order to modify programs based on the features a +user has selected, the target CPU architecture, the target OS, or other parameters under control +of the user. Rust programs may use conditional compilation in these ways: + +* By applying the `#[cfg(c)]` attribute to language elements, where `c` is a condition. +* By applying the `#[cfg_attr(c, attr)]` attribute to language elements, where `c` is a conditional + and `attr` is an attribute to apply. +* By using the `cfg!(c)` built-in macro, where `c` is a condition. The compiler replaces the macro + call with a `true` or `false` literal. + +A _condition_ can take one of two forms: + +* A single identifier, such as `#[cfg(test)]` or `#[cfg(linux)]`. These are Boolean conditions; + they are either enabled or disabled. +* A condition may test whether a given value is present in a named list of values. For example, + `#[cfg(feature = "lighting")]` tests whether the `lighting` feature is enabled. Note that a given + condition name may have any number of enabled values; for example, it is legal to invoke + `rustc --cfg feature="lighting" --cfg feature="bump_maps"`. + +## Checking conditions names + +`rustc` can optionally verify that all condition names used in source code are valid. "Valid" is +distinct from "enabled". A _valid_ condition is one that is allowed to appear in source code; the +condition may be enabled or disabled, but it is still valid. An _enabled_ condition is one which has +been specified with a `--cfg foo` or `--cfg 'foo="value"'` option. + +For example, `rustc` can detect this bug, where the `test` condition is misspelled as `tset`: + +```rust +if cfg!(tset) { // uh oh, should have been 'test' + ... +} +``` + +To catch this error, we give `rustc` the set of valid condition names: + +```bash +rustc --cfg 'valid(name1, name2, ..., nameN)' ... +``` + +The `--cfg 'valid(...)'` option specifies the set of valid condition names. + +### Examples + +```bash +rustc --cfg 'valid(cats, dogs, foo, bar)' ... +``` + +The `--cfg 'valid(...)'` option may be repeated. If it is repeated, then the condition names of +all of the options are merged into a single set. For example: + +```bash +rustc --cfg 'valid(cats)' --cfg 'valid(dogs)' ... +``` + +### Well-known condition names + +`rustc` defines a set of well-known conditions, such as `test`, `target_os`, etc. These conditions +are always valid; it is not necessary to enable checking for these conditions. If these conditions +are specified in a `--cfg valid(...)` option, they will be ignored. This set of well-known names is +a part of the stable interface of the compiler. New well-known conditions may be added in the +future, because adding a new name cannot break existing code. However, a name may not be removed +from the set of well-known names, because doing would be a breaking change. + +These are the well-known conditions: + +* `feature` +* `linux` +* `test` +* `target_os` +* `target_arch` +* `windows` +* TODO: finish enumerating this list + +## Checking key-value conditions + +For conditions that define a list of values, such as `feature`, we want to verify that any +`#[cfg(feature = "v")]` test uses a valid value `v`. We want to detect this kind of bug: + +```rust +if cfg!(feature = "awwwwsome") { // should have been "awesome" + ... +} +``` + +This kind of bug could be due to a typo or a bad PR merge. It could also occur because a feature +was removed from a `Cargo.toml` file, but source code still contains references to it. + +To catch these errors, we give `rustc` the set of valid values for a given condition name. +To do so, we extend the syntax of the `--cfg` option. For a given condition name `c` (such as +`feature`), we specify the set of valid values as: + +```bash +rustc --cfg 'valid_values(c, "value1", "value2", ... "valueN")' +``` + +### Examples + +```bash +rustc --cfg 'valid_values(feature, "derive", "parsing", "printing", "proc-macro")' + +# or equivalently: +rustc --cfg 'valid_values(feature, "derive")' \ + 'valid_values(feature, "parsing")' \ + 'valid_values(feature, "printing")' \ + 'valid_values(feature, "proc-macro")' + +rustc --cfg 'valid_values(target_os, "linux", "macos", "ios", "windows", "android")' + +# specifying values for different names requires more than one --cfg option +rustc --cfg 'valid_values(foo, "red", "green")' --cfg 'valid_values(bar, "up", "down")' +``` + +## Checking is opt-in (disabled by default) + +The default behavior of `rustc` is that conditional compilation names and values are not checked. +This maintains compatibility with existing versions of Cargo and other build systems that might +invoke `rustc` directly. All of the information for checking conditional compilation uses new +syntactic forms of the existing `--cfg` option. + +## Cargo support + +Cargo is ideally positioned to enable checking for `feature` flags, since Cargo knows the set of +valid features. Cargo will invoke `rustc --cfg 'valid_values(feature, "...", ...)'`, so that +checking for features is enabled. Optionally, Cargo could also specify the set of valid condition +names. + +## Command line flags for checking conditional compilation + +To use this feature, you must tell `rustc` to check conditional compilation keys and the set of key +names that are valid. You do this using the `--cfg check(...)` flag. This flag contains a list of +the allowed key names. + +For example: + +```bash +rustc --cfg 'valid(test, target_os, feature, foo, bar)' ... +``` + +> Note: This example shows test and target_os, but it is not necessary to specify these. All keys +> that are well-known to Rustc are always permitted. + +For dictionary keys, it is necessary to specify the set of values that are legal for a specific +dictionary key. For example: + +```bash + --cfg 'allow_values(feature = values("foo", "bar", ..., "zot")' + ``` + +## What do users need to do to use this? + +Most of the time, users will not invoke `rustc` directly. Instead, users run Cargo, which handles +building command-lines for `rustc`. Cargo handles building the lists of valid conditional +compilation keys. + +> This reflects the final goal, which is turning on condition checking in Cargo. However, we will +> need to do a gradual deployment: +> 1. Users on `nightly` builds can run `cargo -Z check-cfg` to opt-in to checking. +> 2. After stabilization, users can opt-in to conditional checking by using `cargo build --check-cfg`. +> 3. Eventually, if the community experience is positive, Cargo could enable this check by default. + +For users who are not using Cargo, they will need to work with their build system to add support +for this. Doing so is out-of-scope for this RFC. + +# Reference-level explanation + +## What Cargo does + +When Cargo builds a `rustc` command line, it knows which features are enabled and which are +disabled. Cargo normally specifies the set of enabled features like so: + +```bash +rustc --cfg 'feature = "lighting"' --cfg 'feature = "bump_maps"' ... +``` + +When conditional compilation checking is enabled, Cargo will also specify which features are +valid, so that `rustc` can validate conditional compilation tests. For example: + +```bash +rustc --cfg 'feature = "lighting"' --cfg 'feature = "bump_maps"' \ + --cfg 'valid(feature, "lighting", "bump_maps", "mip_maps", "vulkan")' +``` + +In this command-line, Cargo has specified the full set of _valid_ features (`lighting`, +`bump_maps`, `mip_maps`, `vulkan`) while also specifying which of those features are currently +_enabled_ (`lighting`, `bump_maps`). + +## Command line arguments reference + +All information would be passed to Rustc by enhancing the forms accepted for the existing `--cfg` +option. To enable checking key names and to specify the set of acceptable key names, use this form: + +```bash +rustc --cfg 'valid(k1, k2, ..., kN)' +``` + +Where `k1..kN` are key names. These are specified as bare identifiers, not quoted strings. If this +option is not specified, then key name checking is not performed. If this option is specified, then +each usage of this option (such as a `#[cfg]` attribute, `#[cfg_attr]` attribute, or `cfg!(...)` +call) is checked against this list. If a key is not present in this list, then a diagnostic is +issued. The default diagnostic level for an invalid key is a warning. This can be promoted to an +error by using `#![deny(invalid_cfg_key)]` or the equivalent command line option. + +If the `valid(...)` option is repeated, then the set of allowed key names is the union of +those specified in all options. + +To enable checking dictionary values, specify the set of allowable values: + +```bash +rustc --cfg 'valid_values(c, "value1", "value2", ... "valueN") +``` + +where `c` is the condition name, such as `feature` or `target_os`, and the `valueN` terms are the +values that are allowed for that condition. The `valid_values` option can be repeated, both for the +same key and for different keys. If it is repeated for the same condition name, then the sets of +values for that condition are merged together. + +> The `valid` and `valid_values` options are independent. The `valid` option controls +> whether condition names are checked, but it does not require that condition values be checked. + +### Valid values can be split across multiple options + +`rustc` processes all `--cfg` options before compiling any code. The valid condition names and +values are the union of all options specified on the command line. For example, this command line: + +```bash +rustc --cfg 'valid_values(feature("lion", "zebra"))' + +# is equivalent to: +rustc --cfg 'valid_values(feature("lion"))' \ + --cfg 'valid_values(feature("zebra"))' +``` + +This is intended to give tool developers more flexibility when generating Rustc command lines. + +### Enabled conditions are implicitly valid + +Specifying an enabled condition name implicitly makes it valid. For example, the following +invocations are equivalent: + +```bash +rustc --cfg 'valid_values(feature, "lion", "zebra")' --cfg 'feature="lion"' + +# equivalent: +rustc --cfg 'valid_values(feature, "zebra")' --cfg 'feature="lion"' +``` + +Specifying a condition value also implicitly marks that condition _name_ as valid. For example, +the following invocations are equivalent: + +```bash +# specifying valid_values(foo, ...) implicitly adds 'foo' to the set of valid condition names +rustc --cfg 'valid(foo, bar)' --cfg 'valid_values(foo, "lion")' --cfg 'foo="lion"' + +# so the above can be simplified to: +rustc --cfg 'valid(bar)' --cfg 'valid_values(foo, "lion")' --cfg 'foo="lion"' +``` + +## Stabilizing + +Until this feature is stabilized, it can only be used with a `nightly` compiler, and only when +specifying the `rustc -Z check-cfg ...` option. + +Similarly, users of `nightly` Cargo builds must also opt-in to use this feature, by specifying +`cargo build -Z check-cfg ...`. + +Experience gained during stabilization will determine how this feature is best enabled in the final +product. Ideally, once the feature is stabilized in `rustc`, the `-Z check-cfg` requirement will +be dropped from `rustc`. Stabilizing in Cargo may require a stable opt-in flag, however. + +## Diagnostics + +Conditional checking can report these diagnostics: + +* `invalid_cfg_name`: Indicates that a condition name was not in the set of valid names. + This diagnostic will only be reported if the command line options enable checking condition names + (i.e. there is at least one `--cfg 'allowed(...)' option` and an invalid condition name is found + during compilation. + +* `invalid_cfg_value`: Indicates that source code contained a condition value that was invalid. + This diagnostic will only be reported if the command line options enable checking condition values + for the specified condition name (i.e. there is a least one `--cfg 'allowed_values(c, ...)'` for + a given condition name `c`). + +All of the diagnostics defined by this RFC are reported as warnings. They can be upgraded to +errors or silenced using the usual diagnostics controls. + +## Examples + +Consider this command line: + +```bash +rustc --cfg 'valid(feature)' \ + --cfg 'valid_values(feature, "lion", "zebra")' \ + --cfg 'feature = "lion"' + example.rs +``` + +This command line indicates that this crate has two features: `lion` and `zebra`. The `lion` +feature is enabled, while the `zebra` feature is disabled. Consider compiling this code: + +```rust +// this is valid, and tame_lion() will be compiled +#[cfg(feature = "lion")] +fn tame_lion(lion: Lion) { ... } + +// this is valid, and ride_zebra() will NOT be compiled +#[cfg(feature = "zebra")] +fn ride_zebra(zebra: Zebra) { ... } + +// this is INVALID, and will cause a compiler error +#[cfg(feature = "platypus")] +fn poke_platypus() { ... } + +// this is INVALID, because 'feechure' is not a known key, +// and will cause a compiler error. +#[cfg(feechure = "lion")] +fn tame_lion() { ... } +``` + +> Note: The `--cfg valid(feature)` argument is necessary only to enable checking the condition +> name, as in the last example. `feature` is a well-known key, and so it is not necessary to +> specify it in a `--cfg 'valid(...)'` option. That option can be shorted to `--cfg valid()` in +> order to enable checking condition names. + +## Drawbacks +There are no known drawbacks to this proposal. + +## Rationale and alternatives +This design enables checking for a class of bugs at compile time, rather than detecting them by +running code. + +This design does not break any existing usage of Rustc. It does not change the meaning of existing +programs or existing Rustc command-line options. It is strictly opt-in. If the verification that +this feature provides is valuable, then it could be promoted to a warning in the future, or +eventually an error. There would need to be a cleanup period, though, where we detected failures in +existing crates and fixed them. + +The impact of not doing this is that a class of bugs may go undetected. These bugs are often easy +to find in relatively small systems of code, but experience shows that these kinds of bugs are much +harder to verify in large code bases. Rust should enable developers to scale up from small to large +systems, without losing agility or reliability. + +## Prior art + +Rust has a very strong focus on finding defects at compile time, rather than allowing defects to be +detected much later in the development cycle. Statically checking that conditional compilation is +used correctly is consistent with this approach. + +Many languages have similar facilities for conditional compilation. C, C++, C#, and many of their +variants make extensive use of conditional compilation. The author is unaware of any effort to +systematically verify the correct usage of conditional compilation in these languages. + +## Unresolved questions + +During the RFC process, I expect to resolve the question of what the exact rustc command-line +parameters should be, at least enough to enable the feature for nightly builds. We should avoid +bikeshedding on the exact syntax, but should agree on the semantics. We should agree on what is +checked, what is not checked, how checking is enabled, and how it is performed. We should agree on +what information is passed from Cargo to Rustc. + +During the implementation and before stabilization, I expect to resolve the question of how many errors this actually detects. How many crates on crates.io actually have invalid `#[cfg]` usage? +How valuable is this feature? + +One possible source of problems may come from build scripts (`build.rs` files) that add `--cfg` +options that Cargo is not aware of. For exaple, if a `Cargo.toml` file did _not_ define a feature +flag of `foo`, but the `build.rs` file added a `--cfg feature="foo"` option, then source code +could use `foo` in a condition. My guess is that this is rare, and that a Crater run will expose +this kind of problem. + +# Future possibilities + +* Should these checks be enabled by default in Cargo? +* How many public crates would fail these checks? +* If these checks are enabled by default in Cargo, should they be warnings or errors? From 3e5f1567bcc9c9a71ca62dae75bb2366b6129c6b Mon Sep 17 00:00:00 2001 From: Arlie Davis Date: Tue, 3 Nov 2020 16:14:53 -0800 Subject: [PATCH 02/12] PR feedback --- text/0000-conditional-compilation-checking.md | 80 +++++++++---------- 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md index 2dea1e8fcf0..c6d2bed18b3 100644 --- a/text/0000-conditional-compilation-checking.md +++ b/text/0000-conditional-compilation-checking.md @@ -157,31 +157,27 @@ names. ## Command line flags for checking conditional compilation -To use this feature, you must tell `rustc` to check conditional compilation keys and the set of key -names that are valid. You do this using the `--cfg check(...)` flag. This flag contains a list of -the allowed key names. - -For example: +To use this feature, you give `rustc` the set of condition names that are valid, by using the +`--cfg valid(...)` flag. For example: ```bash rustc --cfg 'valid(test, target_os, feature, foo, bar)' ... ``` -> Note: This example shows test and target_os, but it is not necessary to specify these. All keys -> that are well-known to Rustc are always permitted. +> Note: This example shows `test` and `target_os`, but it is not necessary to specify these. All +> condition names that are well-known to `rustc` are always permitted. -For dictionary keys, it is necessary to specify the set of values that are legal for a specific -dictionary key. For example: +For condition values, specify the set of values that are legal for a given condition name. For +example: ```bash - --cfg 'allow_values(feature = values("foo", "bar", ..., "zot")' + --cfg 'valid_values(feature, "foo", "bar", ..., "zot")' ``` ## What do users need to do to use this? Most of the time, users will not invoke `rustc` directly. Instead, users run Cargo, which handles -building command-lines for `rustc`. Cargo handles building the lists of valid conditional -compilation keys. +building command-lines for `rustc`. Cargo handles building the lists of valid condition names. > This reflects the final goal, which is turning on condition checking in Cargo. However, we will > need to do a gradual deployment: @@ -200,14 +196,14 @@ When Cargo builds a `rustc` command line, it knows which features are enabled an disabled. Cargo normally specifies the set of enabled features like so: ```bash -rustc --cfg 'feature = "lighting"' --cfg 'feature = "bump_maps"' ... +rustc --cfg 'feature="lighting"' --cfg 'feature="bump_maps"' ... ``` When conditional compilation checking is enabled, Cargo will also specify which features are valid, so that `rustc` can validate conditional compilation tests. For example: ```bash -rustc --cfg 'feature = "lighting"' --cfg 'feature = "bump_maps"' \ +rustc --cfg 'feature="lighting"' --cfg 'feature="bump_maps"' \ --cfg 'valid(feature, "lighting", "bump_maps", "mip_maps", "vulkan")' ``` @@ -218,32 +214,32 @@ _enabled_ (`lighting`, `bump_maps`). ## Command line arguments reference All information would be passed to Rustc by enhancing the forms accepted for the existing `--cfg` -option. To enable checking key names and to specify the set of acceptable key names, use this form: +option. To enable checking condition names and to specify the set of valid names, use this form: ```bash -rustc --cfg 'valid(k1, k2, ..., kN)' +rustc --cfg 'valid(c1, c2, ..., cN)' ``` -Where `k1..kN` are key names. These are specified as bare identifiers, not quoted strings. If this -option is not specified, then key name checking is not performed. If this option is specified, then -each usage of this option (such as a `#[cfg]` attribute, `#[cfg_attr]` attribute, or `cfg!(...)` -call) is checked against this list. If a key is not present in this list, then a diagnostic is -issued. The default diagnostic level for an invalid key is a warning. This can be promoted to an -error by using `#![deny(invalid_cfg_key)]` or the equivalent command line option. +Where `c1..cN` are condition names. These are specified as bare identifiers, not quoted strings. If +this option is not specified, then condition checking is not performed. If this option is specified, +then each usage of this option (such as a `#[cfg]` attribute, `#[cfg_attr]` attribute, or +`cfg!(...)` call) is checked against this list. If a name is not present in this list, then a +diagnostic is issued. The default diagnostic level for an invalid name is a warning. This can be +promoted to an error by using `#![deny(invalid_cfg_name)]` or the equivalent command line option. -If the `valid(...)` option is repeated, then the set of allowed key names is the union of +If the `valid(...)` option is repeated, then the set of valid condition names is the union of those specified in all options. -To enable checking dictionary values, specify the set of allowable values: +To enable checking condition values, specify the set of valid values: ```bash -rustc --cfg 'valid_values(c, "value1", "value2", ... "valueN") +rustc --cfg 'valid_values(c,"value1","value2", ... "valueN") ``` where `c` is the condition name, such as `feature` or `target_os`, and the `valueN` terms are the -values that are allowed for that condition. The `valid_values` option can be repeated, both for the -same key and for different keys. If it is repeated for the same condition name, then the sets of -values for that condition are merged together. +values that are valid for that condition. The `valid_values` 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. > The `valid` and `valid_values` options are independent. The `valid` option controls > whether condition names are checked, but it does not require that condition values be checked. @@ -254,11 +250,11 @@ values for that condition are merged together. values are the union of all options specified on the command line. For example, this command line: ```bash -rustc --cfg 'valid_values(feature("lion", "zebra"))' +rustc --cfg 'valid_values(feature,"lion","zebra")' # is equivalent to: -rustc --cfg 'valid_values(feature("lion"))' \ - --cfg 'valid_values(feature("zebra"))' +rustc --cfg 'valid_values(feature,"lion")' \ + --cfg 'valid_values(feature,"zebra")' ``` This is intended to give tool developers more flexibility when generating Rustc command lines. @@ -269,10 +265,10 @@ Specifying an enabled condition name implicitly makes it valid. For example, the invocations are equivalent: ```bash -rustc --cfg 'valid_values(feature, "lion", "zebra")' --cfg 'feature="lion"' +rustc --cfg 'valid_values(feature,"lion","zebra")' --cfg 'feature="lion"' # equivalent: -rustc --cfg 'valid_values(feature, "zebra")' --cfg 'feature="lion"' +rustc --cfg 'valid_values(feature,"zebra")' --cfg 'feature="lion"' ``` Specifying a condition value also implicitly marks that condition _name_ as valid. For example, @@ -280,10 +276,10 @@ the following invocations are equivalent: ```bash # specifying valid_values(foo, ...) implicitly adds 'foo' to the set of valid condition names -rustc --cfg 'valid(foo, bar)' --cfg 'valid_values(foo, "lion")' --cfg 'foo="lion"' +rustc --cfg 'valid(foo,bar)' --cfg 'valid_values(foo,"lion")' --cfg 'foo="lion"' # so the above can be simplified to: -rustc --cfg 'valid(bar)' --cfg 'valid_values(foo, "lion")' --cfg 'foo="lion"' +rustc --cfg 'valid(bar)' --cfg 'valid_values(foo,"lion")' --cfg 'foo="lion"' ``` ## Stabilizing @@ -304,12 +300,12 @@ Conditional checking can report these diagnostics: * `invalid_cfg_name`: Indicates that a condition name was not in the set of valid names. This diagnostic will only be reported if the command line options enable checking condition names - (i.e. there is at least one `--cfg 'allowed(...)' option` and an invalid condition name is found + (i.e. there is at least one `--cfg 'valid(...)'` option and an invalid condition name is found during compilation. * `invalid_cfg_value`: Indicates that source code contained a condition value that was invalid. This diagnostic will only be reported if the command line options enable checking condition values - for the specified condition name (i.e. there is a least one `--cfg 'allowed_values(c, ...)'` for + for the specified condition name (i.e. there is a least one `--cfg 'valid_values(c, ...)'` for a given condition name `c`). All of the diagnostics defined by this RFC are reported as warnings. They can be upgraded to @@ -321,8 +317,8 @@ Consider this command line: ```bash rustc --cfg 'valid(feature)' \ - --cfg 'valid_values(feature, "lion", "zebra")' \ - --cfg 'feature = "lion"' + --cfg 'valid_values(feature,"lion","zebra")' \ + --cfg 'feature="lion"' example.rs ``` @@ -342,15 +338,15 @@ fn ride_zebra(zebra: Zebra) { ... } #[cfg(feature = "platypus")] fn poke_platypus() { ... } -// this is INVALID, because 'feechure' is not a known key, +// this is INVALID, because 'feechure' is not a known condition name, // and will cause a compiler error. #[cfg(feechure = "lion")] fn tame_lion() { ... } ``` > Note: The `--cfg valid(feature)` argument is necessary only to enable checking the condition -> name, as in the last example. `feature` is a well-known key, and so it is not necessary to -> specify it in a `--cfg 'valid(...)'` option. That option can be shorted to `--cfg valid()` in +> name, as in the last example. `feature` is a well-known condition name, and so it is not necessary +> to specify it in a `--cfg 'valid(...)'` option. That option can be shorted to `--cfg valid()` in > order to enable checking condition names. ## Drawbacks From c14e01222d44d1c5f1eb14d62641d0f7beb4dd47 Mon Sep 17 00:00:00 2001 From: Arlie Davis Date: Tue, 3 Nov 2020 16:49:05 -0800 Subject: [PATCH 03/12] PR feedback --- text/0000-conditional-compilation-checking.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md index c6d2bed18b3..b5512a518b6 100644 --- a/text/0000-conditional-compilation-checking.md +++ b/text/0000-conditional-compilation-checking.md @@ -14,8 +14,12 @@ fn do_windows_thing() { /* ... */ } The developer intended to test for the feature named `windows`. This could easily have been detected by `rustc` if it had known the set of all valid `feature` flags, not only the ones currently -enabled. Cargo already has this information, but it does not communicate it to `rustc`. This RFC -proposes a means for `rustc` to check this information. +enabled. + +This RFC proposes adding new command-line options to `rustc`, which will allow Cargo (and other +build tools) to inform `rustc` of the set of valid feature flags. Using conditions that are not +valid will cause a diagnostic warning. This feature is opt-in, for backwards compatibility; +if no valid configuration options are presented to `rustc` then no warnings are generated. # Motivation From ee717ed5f8dad980537a938c74ca998eb759065f Mon Sep 17 00:00:00 2001 From: Arlie Davis Date: Tue, 3 Nov 2020 17:00:05 -0800 Subject: [PATCH 04/12] PR feedback --- text/0000-conditional-compilation-checking.md | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md index b5512a518b6..2da94f2cf8d 100644 --- a/text/0000-conditional-compilation-checking.md +++ b/text/0000-conditional-compilation-checking.md @@ -152,6 +152,87 @@ This maintains compatibility with existing versions of Cargo and other build sys invoke `rustc` directly. All of the information for checking conditional compilation uses new syntactic forms of the existing `--cfg` option. +Checking condition names is independent of checking condition values, for those conditions that +use value lists. Examples: + +### Example: Checking condition names, but not values + +```bash +# This turns on checking for condition names, but not values, such as 'feature' values. +rustc --cfg 'valid(is_embedded, has_feathers)' \ + --cfg has_feathers \ + --cfg 'feature="zapping"' +``` + +```rust +#[cfg(is_embedded)] // this is valid, and #[cfg] evaluates to disabled +fn do_embedded() {} + +#[cfg(has_feathers)] // this is valid, and #[cfg] evalutes to enabled +fn do_features() {} + +#[cfg(has_mumble_frotz)] // this is INVALID +fn do_mumble_frotz() {} + +#[cfg(feature = "lasers")] // this is valid, because valid_values() was never used +fn shoot_lasers() {} +``` + +### Example: Checking feature values, but not condition names + +```bash +# This turns on checking for feature values, but not for condition names. +rustc --cfg 'valid_values(feature, "zapping", "lasers")' \ + --cfg 'feature="zapping"' +``` + +```rust +#[cfg(is_embedded)] // this is valid, because --cfg valid(...) was never used +fn do_embedded() {} + +#[cfg(has_feathers)] // this is valid, because --cfg valid(...) was never used +fn do_features() {} + +#[cfg(has_mumble_frotz)] // this is valid, because --cfg valid(...) was never used +fn do_mumble_frotz() {} + +#[cfg(feature = "lasers")] // this is valid, because "lasers" is in the valid_values(feature) list +fn shoot_lasers() {} + +#[cfg(feature = "monkeys")] // this is INVALID, because "monkeys" is not in + // the valid_values(feature) list +fn write_shakespear() {} +``` + +### Example: Checking both condition names and feature values + +```bash +# This turns on checking for feature values and for condition names. +rustc --cfg 'valid_values(feature, "zapping", "lasers")' \ + --cfg 'valid(is_embedded, has_feathers)' + --cfg has_feathers \ + --cfg 'feature="zapping"' \ +``` + + +```rust +#[cfg(is_embedded)] // this is valid, and #[cfg] evaluates to disabled +fn do_embedded() {} + +#[cfg(has_feathers)] // this is valid, and #[cfg] evalutes to enabled +fn do_features() {} + +#[cfg(has_mumble_frotz)] // this is INVALID +fn do_mumble_frotz() {} + +#[cfg(feature = "lasers")] // this is valid, because "lasers" is in the valid_values(feature) list +fn shoot_lasers() {} + +#[cfg(feature = "monkeys")] // this is INVALID, because "monkeys" is not in + // the valid_values(feature) list +fn write_shakespear() {} +``` + ## Cargo support Cargo is ideally positioned to enable checking for `feature` flags, since Cargo knows the set of From affc381b7fb5c0c0a163f9a713c5b1ca8c7adcf0 Mon Sep 17 00:00:00 2001 From: sivadeilra Date: Wed, 4 Nov 2020 16:28:08 -0800 Subject: [PATCH 05/12] Update text/0000-conditional-compilation-checking.md Co-authored-by: Joshua Nelson --- text/0000-conditional-compilation-checking.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md index 2da94f2cf8d..630540812d0 100644 --- a/text/0000-conditional-compilation-checking.md +++ b/text/0000-conditional-compilation-checking.md @@ -318,7 +318,7 @@ those specified in all options. To enable checking condition values, specify the set of valid values: ```bash -rustc --cfg 'valid_values(c,"value1","value2", ... "valueN") +rustc --cfg 'valid_values(c,"value1","value2", ... "valueN")' ``` where `c` is the condition name, such as `feature` or `target_os`, and the `valueN` terms are the From e6b5dcccbc66c0170fdab44353210203bd89a00f Mon Sep 17 00:00:00 2001 From: Arlie Davis Date: Fri, 6 Nov 2020 14:20:12 -0800 Subject: [PATCH 06/12] PR feedback Added new --check-cfg flag. Rather than changing the behavior of the existing --cfg flag, we enable validation by using --check-cfg. Clarified some special cases, like empty valid name sets. --- text/0000-conditional-compilation-checking.md | 294 ++++++++++-------- 1 file changed, 159 insertions(+), 135 deletions(-) diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md index 630540812d0..0695d774cf2 100644 --- a/text/0000-conditional-compilation-checking.md +++ b/text/0000-conditional-compilation-checking.md @@ -17,9 +17,10 @@ by `rustc` if it had known the set of all valid `feature` flags, not only the on enabled. This RFC proposes adding new command-line options to `rustc`, which will allow Cargo (and other -build tools) to inform `rustc` of the set of valid feature flags. Using conditions that are not -valid will cause a diagnostic warning. This feature is opt-in, for backwards compatibility; -if no valid configuration options are presented to `rustc` then no warnings are generated. +build tools) to inform `rustc` of the set of valid conditions, such as `feature` tests. Using +conditions that are not valid will cause a diagnostic warning. This feature is opt-in, for backwards +compatibility; if no valid configuration options are presented to `rustc` then no warnings are +generated. # Motivation @@ -50,13 +51,14 @@ A _condition_ can take one of two forms: `#[cfg(feature = "lighting")]` tests whether the `lighting` feature is enabled. Note that a given condition name may have any number of enabled values; for example, it is legal to invoke `rustc --cfg feature="lighting" --cfg feature="bump_maps"`. +* Boolean operators on conditions, such as `not(...)`, `all(...)`, and `any(...)`. ## Checking conditions names -`rustc` can optionally verify that all condition names used in source code are valid. "Valid" is -distinct from "enabled". A _valid_ condition is one that is allowed to appear in source code; the +`rustc` can optionally verify that condition names used in source code are valid. _Valid_ is +distinct from _enabled_. A _valid_ condition is one that is allowed to appear in source code; the condition may be enabled or disabled, but it is still valid. An _enabled_ condition is one which has -been specified with a `--cfg foo` or `--cfg 'foo="value"'` option. +been specified with a `--cfg foo` or `--cfg 'foo = "value"'` option. For example, `rustc` can detect this bug, where the `test` condition is misspelled as `tset`: @@ -69,32 +71,23 @@ if cfg!(tset) { // uh oh, should have been 'test' To catch this error, we give `rustc` the set of valid condition names: ```bash -rustc --cfg 'valid(name1, name2, ..., nameN)' ... +rustc --check-cfg 'names(name1, name2, ..., nameN)' ... ``` -The `--cfg 'valid(...)'` option specifies the set of valid condition names. +The `--check-cfg` option does two things: First, it turns on validation for the set of condition +names (and separately for values). Second, it specifies the set of valid condition names (values). -### Examples - -```bash -rustc --cfg 'valid(cats, dogs, foo, bar)' ... -``` - -The `--cfg 'valid(...)'` option may be repeated. If it is repeated, then the condition names of -all of the options are merged into a single set. For example: - -```bash -rustc --cfg 'valid(cats)' --cfg 'valid(dogs)' ... -``` +Like many `rustc` options the `--check-cfg` option can be specified in a single-argument form, with +the option name and its argument joined by `=`, or can be specified in a two-argument form. ### Well-known condition names `rustc` defines a set of well-known conditions, such as `test`, `target_os`, etc. These conditions are always valid; it is not necessary to enable checking for these conditions. If these conditions -are specified in a `--cfg valid(...)` option, they will be ignored. This set of well-known names is -a part of the stable interface of the compiler. New well-known conditions may be added in the -future, because adding a new name cannot break existing code. However, a name may not be removed -from the set of well-known names, because doing would be a breaking change. +are specified in a `--check-cfg names(...)` option then they will be ignored. This set of well-known +names is a part of the stable interface of the compiler. New well-known conditions may be added in +the future, because adding a new name cannot break existing code. However, a name may not be removed +from the set of well-known names, because doing so would be a breaking change. These are the well-known conditions: @@ -104,7 +97,7 @@ These are the well-known conditions: * `target_os` * `target_arch` * `windows` -* TODO: finish enumerating this list +* TODO: finish enumerating this list during implementation ## Checking key-value conditions @@ -118,31 +111,18 @@ if cfg!(feature = "awwwwsome") { // should have been "awesome" ``` This kind of bug could be due to a typo or a bad PR merge. It could also occur because a feature -was removed from a `Cargo.toml` file, but source code still contains references to it. +was removed from a `Cargo.toml` file, but source code still contains references to it. Or, a +feature name may have been renamed in one branch, while a new use of that feature was added in +a second branch. We want to catch that kind of accident during a merge. -To catch these errors, we give `rustc` the set of valid values for a given condition name. -To do so, we extend the syntax of the `--cfg` option. For a given condition name `c` (such as -`feature`), we specify the set of valid values as: +To catch these errors, we give `rustc` the set of valid values for a given condition name, by +specifying the `--check-cfg` option. For example: ```bash -rustc --cfg 'valid_values(c, "value1", "value2", ... "valueN")' -``` - -### Examples - -```bash -rustc --cfg 'valid_values(feature, "derive", "parsing", "printing", "proc-macro")' - -# or equivalently: -rustc --cfg 'valid_values(feature, "derive")' \ - 'valid_values(feature, "parsing")' \ - 'valid_values(feature, "printing")' \ - 'valid_values(feature, "proc-macro")' - -rustc --cfg 'valid_values(target_os, "linux", "macos", "ios", "windows", "android")' +rustc --check-cfg 'values(feature, "derive", "parsing", "printing", "proc-macro")' ... # specifying values for different names requires more than one --cfg option -rustc --cfg 'valid_values(foo, "red", "green")' --cfg 'valid_values(bar, "up", "down")' +rustc --check-cfg 'values(foo, "red", "green")' --check-cfg 'values(bar, "up", "down")' ``` ## Checking is opt-in (disabled by default) @@ -153,15 +133,15 @@ invoke `rustc` directly. All of the information for checking conditional compila syntactic forms of the existing `--cfg` option. Checking condition names is independent of checking condition values, for those conditions that -use value lists. Examples: +use value lists. ### Example: Checking condition names, but not values ```bash # This turns on checking for condition names, but not values, such as 'feature' values. -rustc --cfg 'valid(is_embedded, has_feathers)' \ +rustc --check-cfg 'names(is_embedded, has_feathers)' \ --cfg has_feathers \ - --cfg 'feature="zapping"' + --cfg 'feature = "zapping"' ``` ```rust @@ -174,7 +154,7 @@ fn do_features() {} #[cfg(has_mumble_frotz)] // this is INVALID fn do_mumble_frotz() {} -#[cfg(feature = "lasers")] // this is valid, because valid_values() was never used +#[cfg(feature = "lasers")] // this is valid, because values() was never used fn shoot_lasers() {} ``` @@ -182,96 +162,79 @@ fn shoot_lasers() {} ```bash # This turns on checking for feature values, but not for condition names. -rustc --cfg 'valid_values(feature, "zapping", "lasers")' \ +rustc --check-cfg 'values(feature, "zapping", "lasers")' \ --cfg 'feature="zapping"' ``` ```rust -#[cfg(is_embedded)] // this is valid, because --cfg valid(...) was never used +#[cfg(is_embedded)] // this is valid, because --check-cfg names(...) was never used fn do_embedded() {} -#[cfg(has_feathers)] // this is valid, because --cfg valid(...) was never used +#[cfg(has_feathers)] // this is valid, because --check-cfg names(...) was never used fn do_features() {} -#[cfg(has_mumble_frotz)] // this is valid, because --cfg valid(...) was never used +#[cfg(has_mumble_frotz)] // this is valid, because --check-cfg names(...) was never used fn do_mumble_frotz() {} -#[cfg(feature = "lasers")] // this is valid, because "lasers" is in the valid_values(feature) list +#[cfg(feature = "lasers")] // this is valid, because "lasers" is in the + // --check-cfg values(feature) list fn shoot_lasers() {} -#[cfg(feature = "monkeys")] // this is INVALID, because "monkeys" is not in - // the valid_values(feature) list -fn write_shakespear() {} +#[cfg(feature = "monkeys")] // this is INVALID, because "monkeys" is not in the + // --check-cfg values(feature) list +fn write_shakespeare() {} ``` ### Example: Checking both condition names and feature values ```bash # This turns on checking for feature values and for condition names. -rustc --cfg 'valid_values(feature, "zapping", "lasers")' \ - --cfg 'valid(is_embedded, has_feathers)' +rustc --check-cfg 'names(is_embedded, has_feathers)' \ + --check-cfg 'values(feature, "zapping", "lasers")' \ --cfg has_feathers \ --cfg 'feature="zapping"' \ ``` - ```rust -#[cfg(is_embedded)] // this is valid, and #[cfg] evaluates to disabled +#[cfg(is_embedded)] // this is valid, and #[cfg] evaluates to disabled fn do_embedded() {} -#[cfg(has_feathers)] // this is valid, and #[cfg] evalutes to enabled +#[cfg(has_feathers)] // this is valid, and #[cfg] evalutes to enabled fn do_features() {} -#[cfg(has_mumble_frotz)] // this is INVALID +#[cfg(has_mumble_frotz)] // this is INVALID, because has_mumble_frotz is not in the + // --check-cfg names(...) list fn do_mumble_frotz() {} -#[cfg(feature = "lasers")] // this is valid, because "lasers" is in the valid_values(feature) list +#[cfg(feature = "lasers")] // this is valid, because "lasers" is in the values(feature) list fn shoot_lasers() {} #[cfg(feature = "monkeys")] // this is INVALID, because "monkeys" is not in - // the valid_values(feature) list + // the values(feature) list fn write_shakespear() {} ``` ## Cargo support Cargo is ideally positioned to enable checking for `feature` flags, since Cargo knows the set of -valid features. Cargo will invoke `rustc --cfg 'valid_values(feature, "...", ...)'`, so that +valid features. Cargo will invoke `rustc --check-cfg 'values(feature, "...", ...)'`, so that checking for features is enabled. Optionally, Cargo could also specify the set of valid condition names. -## Command line flags for checking conditional compilation - -To use this feature, you give `rustc` the set of condition names that are valid, by using the -`--cfg valid(...)` flag. For example: - -```bash -rustc --cfg 'valid(test, target_os, feature, foo, bar)' ... -``` - -> Note: This example shows `test` and `target_os`, but it is not necessary to specify these. All -> condition names that are well-known to `rustc` are always permitted. - -For condition values, specify the set of values that are legal for a given condition name. For -example: - -```bash - --cfg 'valid_values(feature, "foo", "bar", ..., "zot")' - ``` - -## What do users need to do to use this? - -Most of the time, users will not invoke `rustc` directly. Instead, users run Cargo, which handles -building command-lines for `rustc`. Cargo handles building the lists of valid condition names. +Cargo users will not need to do anything to take advantage of this feature. Cargo will always +specify the set of valid `feature` flags. This may cause warnings in crates that contain invalid +`#[cfg]` conditions. (Rust is permitted to add new lints; new lints are not considered a breaking +change.) If a user upgrades to a version of Cargo / Rust that supports validating features, and +their crate now reports errors, then they will need to align their source code with their +`Cargo.toml` file in order to fix the error. (Or use `#[allow(...)]` to suppress it.) This is a +benefit, because it exposes potential existing bugs. -> This reflects the final goal, which is turning on condition checking in Cargo. However, we will -> need to do a gradual deployment: -> 1. Users on `nightly` builds can run `cargo -Z check-cfg` to opt-in to checking. -> 2. After stabilization, users can opt-in to conditional checking by using `cargo build --check-cfg`. -> 3. Eventually, if the community experience is positive, Cargo could enable this check by default. +## Supporting build systems other than Cargo -For users who are not using Cargo, they will need to work with their build system to add support -for this. Doing so is out-of-scope for this RFC. +Some users invoke `rustc` using build systems other than Cargo. In this case, `rustc` will provide +the mechanism for validating conditions, but those build systems will need to be updated in order +to take advantage of this feature. Doing so is expected to be easy and non-disruptive, since this +feature does not change the meaning of the existing `--cfg` option. # Reference-level explanation @@ -289,7 +252,7 @@ valid, so that `rustc` can validate conditional compilation tests. For example: ```bash rustc --cfg 'feature="lighting"' --cfg 'feature="bump_maps"' \ - --cfg 'valid(feature, "lighting", "bump_maps", "mip_maps", "vulkan")' + --check-cfg 'values(feature, "lighting", "bump_maps", "mip_maps", "vulkan")' ``` In this command-line, Cargo has specified the full set of _valid_ features (`lighting`, @@ -298,48 +261,95 @@ _enabled_ (`lighting`, `bump_maps`). ## Command line arguments reference -All information would be passed to Rustc by enhancing the forms accepted for the existing `--cfg` -option. To enable checking condition names and to specify the set of valid names, use this form: +`rustc` accepts the `--check-cfg` option, which specifies whether to check conditions and how to +check them. The `--check-cfg` option takes a value, called the _check cfg specification_. The +check cfg specification is parsed using the Rust metadata syntax, just as the `--cfg` option is. +(This allows for easy future extensibility, and for easily specifying moderately-complex data.) + +Each `--check-cfg` option can take one of two forms: + +1. `--check-cfg names(...)` enables checking condition names. +2. `--check-cfg values(...)` enables checking the values within list-valued conditions. + +### The `names(...)` form + + +This form uses a named metadata list: ```bash -rustc --cfg 'valid(c1, c2, ..., cN)' +rustc --check-cfg 'names(name1, name2, ... nameN)' ``` -Where `c1..cN` are condition names. These are specified as bare identifiers, not quoted strings. If -this option is not specified, then condition checking is not performed. If this option is specified, -then each usage of this option (such as a `#[cfg]` attribute, `#[cfg_attr]` attribute, or -`cfg!(...)` call) is checked against this list. If a name is not present in this list, then a -diagnostic is issued. The default diagnostic level for an invalid name is a warning. This can be -promoted to an error by using `#![deny(invalid_cfg_name)]` or the equivalent command line option. +where each `name` is a bare identifier (has no quotes). The order of the names is not significant. + +If `--check-cfg names(...)` is specified at least once, then `rustc` will check all references to +condition names. `rustc` will check every `#[cfg]` attribute, `#[cfg_attr]` attribute, and +`cfg!(...)` call against the provided list of valid condition names. If a name is not present in +this list, then `rustc` will report an `invalid_cfg_name` lint diagnostic. The default diagnostic +level for this lint is `Warn`. + +If `--check-cfg names(...)` is not specified, then `rustc` will not check references to condition +names. -If the `valid(...)` option is repeated, then the set of valid condition names is the union of -those specified in all options. +`--check-cfg names(...)` may be specified more than once. The result is that the list of valid +condition names is merged across all options. It is legal for a condition name to be specified +more than once; redundantly specifying a condition name has no effect. -To enable checking condition values, specify the set of valid values: +To enable checking condition names with an empty set of valid condition names, use the following +form. The parentheses are required. ```bash -rustc --cfg 'valid_values(c,"value1","value2", ... "valueN")' +rustc --check-cfg 'names()' ``` -where `c` is the condition name, such as `feature` or `target_os`, and the `valueN` terms are the -values that are valid for that condition. The `valid_values` 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. +Conditions that are enabled are implicitly valid; it is unnecessary (but legal) to specify a +condition name as both enabled and valid. For example, the following invocations are equivalent: + +### The `values(...)` form + +The `values(...)` form enables checking the values within list-valued conditions. It has this +form: + +```bash +rustc --check-cfg `values(name, "value1", "value2", ... "valueN")' +``` + +where `name` is a bare identifier (has no quotes) and each `"value"` term is a quoted literal +string. `name` specifies the name of the condition, such as `feature` or `target_os`. + +When the `values(...)` option is specified, `rustc` will check every `#[cfg(name = "value")]` +attribute, `#[cfg_attr(name = "value")]` attribute, and `cfg!(name = "value")` call. It will +check that the `"value"` specified is present in the list of valid values. If `"value"` is not +valid, then `rustc` will report an `invalid_cfg_name` lint diagnostic. The default diagnostic +level for this lint is `Warn`. -> The `valid` and `valid_values` options are independent. The `valid` option controls -> whether condition names are checked, but it does not require that condition values be checked. +The form `values()` is an error, because it does not specify a condition name. + +To enable checking of values, but to provide an empty set of valid values, use this form: + +```bash +rustc --check-cfg `values(name)` +``` + +The `--check-cfg values(...)` 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. + +> The `--check-cfg names(...)` and `--check-cfg values(...)` options are independent. `names` +> checks the namespace of condition names; `values` checks the namespace of the values of +> list-valued conditions. ### Valid values can be split across multiple options -`rustc` processes all `--cfg` options before compiling any code. The valid condition names and -values are the union of all options specified on the command line. For example, this command line: +The valid condition values are the union of all options specified on the command line. +For example, this command line: ```bash -rustc --cfg 'valid_values(feature,"lion","zebra")' +# legal but redundant: +rustc --check-cfg 'values(animals, "lion")' --check-cfg 'values(animals, "zebra")' -# is equivalent to: -rustc --cfg 'valid_values(feature,"lion")' \ - --cfg 'valid_values(feature,"zebra")' +# equivalent: +rustc --check-cfg 'values(animals, "lion", "zebra")' ``` This is intended to give tool developers more flexibility when generating Rustc command lines. @@ -350,21 +360,33 @@ Specifying an enabled condition name implicitly makes it valid. For example, the invocations are equivalent: ```bash -rustc --cfg 'valid_values(feature,"lion","zebra")' --cfg 'feature="lion"' +# legal but redundant: +rustc --check-cfg 'values(animals, "lion", "zebra")' --cfg 'animals = "lion"' # equivalent: -rustc --cfg 'valid_values(feature,"zebra")' --cfg 'feature="lion"' +rustc --check-cfg 'values(animals, "zebra")' --cfg 'animals = "lion"' +``` + +Specifying an enabled condition _value_ implicitly makes that _value_ valid. For example, the +following invocations are equivalent: + +```bash +# legal but redundant +rustc --check-cfg 'values(animals, "lion", "zebra")' --cfg 'animals = "lion"' + +# equivalent +rustc --check-cfg 'values(animals, "zebra")' --cfg 'animals = "lion"' ``` Specifying a condition value also implicitly marks that condition _name_ as valid. For example, the following invocations are equivalent: ```bash -# specifying valid_values(foo, ...) implicitly adds 'foo' to the set of valid condition names -rustc --cfg 'valid(foo,bar)' --cfg 'valid_values(foo,"lion")' --cfg 'foo="lion"' +# legal but redundant: +rustc --check-cfg 'names(other, animals)' --check-cfg 'values(animals, "lion")' # so the above can be simplified to: -rustc --cfg 'valid(bar)' --cfg 'valid_values(foo,"lion")' --cfg 'foo="lion"' +rustc --check-cfg 'names(other)' --check-cfg 'values(animals, "lion")' ``` ## Stabilizing @@ -385,12 +407,12 @@ Conditional checking can report these diagnostics: * `invalid_cfg_name`: Indicates that a condition name was not in the set of valid names. This diagnostic will only be reported if the command line options enable checking condition names - (i.e. there is at least one `--cfg 'valid(...)'` option and an invalid condition name is found + (i.e. there is at least one `--cfg 'names(...)'` option and an invalid condition name is found during compilation. * `invalid_cfg_value`: Indicates that source code contained a condition value that was invalid. This diagnostic will only be reported if the command line options enable checking condition values - for the specified condition name (i.e. there is a least one `--cfg 'valid_values(c, ...)'` for + for the specified condition name (i.e. there is a least one `--check-cfg 'values(c, ...)'` for a given condition name `c`). All of the diagnostics defined by this RFC are reported as warnings. They can be upgraded to @@ -401,8 +423,8 @@ errors or silenced using the usual diagnostics controls. Consider this command line: ```bash -rustc --cfg 'valid(feature)' \ - --cfg 'valid_values(feature,"lion","zebra")' \ +rustc --check-cfg 'name(feature)' \ + --check-cfg 'values(feature,"lion","zebra")' \ --cfg 'feature="lion"' example.rs ``` @@ -429,15 +451,17 @@ fn poke_platypus() { ... } fn tame_lion() { ... } ``` -> Note: The `--cfg valid(feature)` argument is necessary only to enable checking the condition -> name, as in the last example. `feature` is a well-known condition name, and so it is not necessary -> to specify it in a `--cfg 'valid(...)'` option. That option can be shorted to `--cfg valid()` in -> order to enable checking condition names. +> Note: The `--check-cfg names(feature)` option is necessary only to enable checking the condition +> name, as in the last example. `feature` is a well-known (always-valid) condition name, and so it +> is not necessary to specify it in a `--check-cfg 'names(...)'` option. That option can be +> shortened to > `--check-cfg names()` in order to enable checking condition names. ## Drawbacks + There are no known drawbacks to this proposal. ## Rationale and alternatives + This design enables checking for a class of bugs at compile time, rather than detecting them by running code. From 1788dd4d201fe34472643088aef8e8c83dbffbf4 Mon Sep 17 00:00:00 2001 From: Arlie Davis Date: Fri, 13 Nov 2020 07:46:34 -0800 Subject: [PATCH 07/12] PR feedback --- text/0000-conditional-compilation-checking.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md index 0695d774cf2..7a2175de39b 100644 --- a/text/0000-conditional-compilation-checking.md +++ b/text/0000-conditional-compilation-checking.md @@ -458,7 +458,12 @@ fn tame_lion() { ... } ## Drawbacks -There are no known drawbacks to this proposal. +* Adds complexity, in the form of additional command-line options. +* As with all lints, correct code may be trigger lints. Developers will need to take time to + examine them and see whether they are legitimate or not. +* To take full advantage of this, build systems (including but not limited to Cargo) must be + updated. However, for those systems that are not updated, there is no penalty or drawback, + since `--check-cfg` is opt-in. ## Rationale and alternatives From dbc02dc3c0ae624259d00b30c8e70003a57efb90 Mon Sep 17 00:00:00 2001 From: Arlie Davis Date: Thu, 14 Jan 2021 20:03:45 -0800 Subject: [PATCH 08/12] Address PR feedback --- text/0000-conditional-compilation-checking.md | 63 +++++++++++++++++-- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md index 7a2175de39b..fd3f60c2ade 100644 --- a/text/0000-conditional-compilation-checking.md +++ b/text/0000-conditional-compilation-checking.md @@ -273,7 +273,6 @@ Each `--check-cfg` option can take one of two forms: ### The `names(...)` form - This form uses a named metadata list: ```bash @@ -302,9 +301,32 @@ form. The parentheses are required. rustc --check-cfg 'names()' ``` +Note that `--check-cfg 'names()'` is _not_ equivalent to omitting the option entirely. +The first form enables checking condition names, while specifying that there are no valid +condition names (outside of the set of well-known names defined by `rustc`). Omitting the +`--check-cfg 'names(...)'` option does not enable checking condition names. + Conditions that are enabled are implicitly valid; it is unnecessary (but legal) to specify a condition name as both enabled and valid. For example, the following invocations are equivalent: +```bash +# condition names will be checked, and 'has_time_travel' is valid +rustc --cfg 'has_time_travel' --check-cfg 'names()' + +# condition names will be checked, and 'has_time_travel' is valid +rustc --cfg 'has_time_travel' --check-cfg 'names(has_time_travel)' +``` + +In contrast, the following two invocations are _not_ equivalent: + +```bash +# condition names will not be checked (because there is no --check-cfg names(...)) +rustc --cfg 'has_time_travel' + +# condition names will be checked, and 'has_time_travel' is both valid and enabled. +rustc --cfg 'has_time_travel' --check-cfg 'names(has_time_travel)' +``` + ### The `values(...)` form The `values(...)` form enables checking the values within list-valued conditions. It has this @@ -320,7 +342,7 @@ string. `name` specifies the name of the condition, such as `feature` or `target When the `values(...)` option is specified, `rustc` will check every `#[cfg(name = "value")]` attribute, `#[cfg_attr(name = "value")]` attribute, and `cfg!(name = "value")` call. It will check that the `"value"` specified is present in the list of valid values. If `"value"` is not -valid, then `rustc` will report an `invalid_cfg_name` lint diagnostic. The default diagnostic +valid, then `rustc` will report an `invalid_cfg_value` lint diagnostic. The default diagnostic level for this lint is `Warn`. The form `values()` is an error, because it does not specify a condition name. @@ -354,19 +376,21 @@ rustc --check-cfg 'values(animals, "lion", "zebra")' This is intended to give tool developers more flexibility when generating Rustc command lines. -### Enabled conditions are implicitly valid +### Enabled condition names are implicitly valid Specifying an enabled condition name implicitly makes it valid. For example, the following invocations are equivalent: ```bash # legal but redundant: -rustc --check-cfg 'values(animals, "lion", "zebra")' --cfg 'animals = "lion"' +rustc --check-cfg 'names(animals)' --cfg 'animals = "lion"' # equivalent: -rustc --check-cfg 'values(animals, "zebra")' --cfg 'animals = "lion"' +rustc --check-cfg 'names()' --cfg 'animals = "lion"' ``` +### Enabled condition values are implicitly valid + Specifying an enabled condition _value_ implicitly makes that _value_ valid. For example, the following invocations are equivalent: @@ -389,6 +413,32 @@ rustc --check-cfg 'names(other, animals)' --check-cfg 'values(animals, "lion")' rustc --check-cfg 'names(other)' --check-cfg 'values(animals, "lion")' ``` +### Checking condition names and values is independent + +Checking condition names may be enabled independently of checking condition values. +If checking of condition values is enabled, then it is enabled separately for each condition name. + +Examples: + +```bash + +# no checking is performed +rustc + +# names are checked, but values are not checked +rustc --check-cfg 'names(has_time_travel)' + +# names are not checked, but 'feature' values are checked. +# note that #[cfg(market = "...")] values are not checked. +rustc --check-cfg 'values(feature, "lighting", "bump_maps")' + +# names are not checked, but 'feature' values _and_ 'market' values are checked. +rustc --check-cfg 'values(feature, "lighting", "bump_maps")' --check-cfg 'markets(feature = "europe", "asia")' + +# names _and_ feature values are checked. +rustc --check-cfg 'names(has_time_travel)' --check-cfg 'values(feature, "lighting", "bump_maps")' +``` + ## Stabilizing Until this feature is stabilized, it can only be used with a `nightly` compiler, and only when @@ -458,7 +508,8 @@ fn tame_lion() { ... } ## Drawbacks -* Adds complexity, in the form of additional command-line options. +* Adds complexity, in the form of additional command-line options. Fortunately, this is + complexity that will be mainly be exposed to build systems, such as Cargo. * As with all lints, correct code may be trigger lints. Developers will need to take time to examine them and see whether they are legitimate or not. * To take full advantage of this, build systems (including but not limited to Cargo) must be From 133cd51ee2e35642b0e12a4fc23ad943ca28bfa5 Mon Sep 17 00:00:00 2001 From: Arlie Davis Date: Sun, 31 Jan 2021 16:01:05 -0800 Subject: [PATCH 09/12] Address PR feedback on drawbacks --- text/0000-conditional-compilation-checking.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md index fd3f60c2ade..acf0278ab1e 100644 --- a/text/0000-conditional-compilation-checking.md +++ b/text/0000-conditional-compilation-checking.md @@ -516,6 +516,25 @@ fn tame_lion() { ... } updated. However, for those systems that are not updated, there is no penalty or drawback, since `--check-cfg` is opt-in. +* This lint will not be able to detect invalid `#[cfg]` tests that are within modules that + are not compiled, presumably because an ancestor `mod` is disabled due to a. For example: + + File `lib.rs` (root module): + ```rust + #[cfg(feature = "this_is_disabled_but_valid")] + mod foo + ``` + + File `foo.rs` (nested module): + ```rust + #[cfg(feature = "oooooops_this_feature_is_misspelled_and_invalid")] + mod uh_uh; + ``` + + The invalid `#[cfg]` attribute in `foo.rs` will not be detected, because `foo.rs` was not + read and parsed. This is a minor drawback, and should not prevent users from benefitting + from checking in most common situations. + ## Rationale and alternatives This design enables checking for a class of bugs at compile time, rather than detecting them by From 174816e2ebc4419d44a1215a6886c1c06d86c978 Mon Sep 17 00:00:00 2001 From: Arlie Davis Date: Thu, 11 Feb 2021 15:16:53 -0800 Subject: [PATCH 10/12] Address PR feedback --- text/0000-conditional-compilation-checking.md | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md index acf0278ab1e..ee969d2e8f8 100644 --- a/text/0000-conditional-compilation-checking.md +++ b/text/0000-conditional-compilation-checking.md @@ -433,10 +433,12 @@ rustc --check-cfg 'names(has_time_travel)' rustc --check-cfg 'values(feature, "lighting", "bump_maps")' # names are not checked, but 'feature' values _and_ 'market' values are checked. -rustc --check-cfg 'values(feature, "lighting", "bump_maps")' --check-cfg 'markets(feature = "europe", "asia")' +rustc --check-cfg 'values(feature, "lighting", "bump_maps")' \ + --check-cfg 'values(market, "europe", "asia")' # names _and_ feature values are checked. -rustc --check-cfg 'names(has_time_travel)' --check-cfg 'values(feature, "lighting", "bump_maps")' +rustc --check-cfg 'names(has_time_travel)' \ + --check-cfg 'values(feature, "lighting", "bump_maps")' ``` ## Stabilizing @@ -563,14 +565,22 @@ systematically verify the correct usage of conditional compilation in these lang ## Unresolved questions -During the RFC process, I expect to resolve the question of what the exact rustc command-line -parameters should be, at least enough to enable the feature for nightly builds. We should avoid -bikeshedding on the exact syntax, but should agree on the semantics. We should agree on what is -checked, what is not checked, how checking is enabled, and how it is performed. We should agree on -what information is passed from Cargo to Rustc. - -During the implementation and before stabilization, I expect to resolve the question of how many errors this actually detects. How many crates on crates.io actually have invalid `#[cfg]` usage? -How valuable is this feature? +This RFC specifies the exact syntax of this feature in source code and in the +command-line options for `rustc`. However, it does not address how these will be used +by tools, such as Cargo. This is a split between "mechanism" and "policy"; the mechanism +(what goes in `rustc`) is specified in this RFC, but the policies that control this +mechanism are intentionally left out of scope. + +We expect the stabilization process for the mechanism (the support in `rustc`) to stabilize +relatively quickly. Separately, over a much longer time frame, we expect the polices that +control those options to stabilize more slowly. For example, it seems uncontroversial for +Cargo to enable checking for `feature = "..."` values immediately; this could be +implemented and stabilized quickly. + +However, when (if ever) should Cargo enable checking condition _names_? For crates that +do not have a `build.rs` script, Cargo could enable checking condition names immediately. +But for crates that do have a `build.rs` script, we may need a way for those scripts to +control the behavior of checking condition names. One possible source of problems may come from build scripts (`build.rs` files) that add `--cfg` options that Cargo is not aware of. For exaple, if a `Cargo.toml` file did _not_ define a feature From 858d9404256cc5b68741f660805de25480c0427c Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 23 Feb 2021 10:12:50 -0800 Subject: [PATCH 11/12] Update 3013 header. --- text/0000-conditional-compilation-checking.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/text/0000-conditional-compilation-checking.md b/text/0000-conditional-compilation-checking.md index ee969d2e8f8..c16dfa8f7c6 100644 --- a/text/0000-conditional-compilation-checking.md +++ b/text/0000-conditional-compilation-checking.md @@ -1,3 +1,8 @@ +- Feature Name: N/A +- Start Date: 2020-11-04 +- RFC PR: [rust-lang/rfcs#3013](https://github.com/rust-lang/rfcs/pull/3013) +- Rust Issue: [rust-lang/rust#82450](https://github.com/rust-lang/rust/issues/82450) + # Checking conditional compilation at compile time # Summary @@ -83,7 +88,7 @@ the option name and its argument joined by `=`, or can be specified in a two-arg ### Well-known condition names `rustc` defines a set of well-known conditions, such as `test`, `target_os`, etc. These conditions -are always valid; it is not necessary to enable checking for these conditions. If these conditions +are always valid; it is not necessary to enable checking for these conditions. If these conditions are specified in a `--check-cfg names(...)` option then they will be ignored. This set of well-known names is a part of the stable interface of the compiler. New well-known conditions may be added in the future, because adding a new name cannot break existing code. However, a name may not be removed @@ -486,15 +491,15 @@ feature is enabled, while the `zebra` feature is disabled. Consider compiling th ```rust // this is valid, and tame_lion() will be compiled -#[cfg(feature = "lion")] +#[cfg(feature = "lion")] fn tame_lion(lion: Lion) { ... } // this is valid, and ride_zebra() will NOT be compiled -#[cfg(feature = "zebra")] +#[cfg(feature = "zebra")] fn ride_zebra(zebra: Zebra) { ... } -// this is INVALID, and will cause a compiler error -#[cfg(feature = "platypus")] +// this is INVALID, and will cause a compiler error +#[cfg(feature = "platypus")] fn poke_platypus() { ... } // this is INVALID, because 'feechure' is not a known condition name, @@ -539,7 +544,7 @@ fn tame_lion() { ... } ## Rationale and alternatives -This design enables checking for a class of bugs at compile time, rather than detecting them by +This design enables checking for a class of bugs at compile time, rather than detecting them by running code. This design does not break any existing usage of Rustc. It does not change the meaning of existing From d2d74f9087236b867a83231bb71fb10783b359b3 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 23 Feb 2021 10:14:17 -0800 Subject: [PATCH 12/12] Merging RFC 3013 --- ...ation-checking.md => 3013-conditional-compilation-checking.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename text/{0000-conditional-compilation-checking.md => 3013-conditional-compilation-checking.md} (100%) diff --git a/text/0000-conditional-compilation-checking.md b/text/3013-conditional-compilation-checking.md similarity index 100% rename from text/0000-conditional-compilation-checking.md rename to text/3013-conditional-compilation-checking.md