mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #119930 - Urgau:check-cfg-empty-values-means-empty, r=petrochenkov

Browse Source 
Add way to express that no values are expected with check-cfg

This PR adds way to express no-values (no values expected) with `--check-cfg` by making empty `values()` no longer mean `values(none())` (internal: `&[None]`) and now be an empty list (internal: `&[]`).

### Context

Currently `--check-cfg` has a way to express that _any value is expected_ with `values(any())`, but has no way to do the inverse and say that _no value is expected_.

This would be particularly useful for build systems that control a config name and it's values as they could always declare a config name as expected and if in the current state they have values pass them and if not pass an empty list.

To give a more concrete example, Cargo `--check-cfg` currently needs to generate:
 - `--check-cfg=cfg(feature, values(...))` for the case with declared features
 - and `--check-cfg=cfg()` for the case without any features declared

This means that when there are no features declared, users will get an `unexpected config name` but from the point of view of Cargo the config name `feature` is expected, it's just that for now there aren't any values for it.

See [Cargo `check_cfg_args` function](92395d9010/src/cargo/core/compiler/mod.rs (L1263-L1281)) for more details.

### De-specializing *empty* `values()`

To solve this issue I propose that we "de-specialize" empty `values()` to no longer mean `values(none())` but to actually mean empty set/list. This is one of the last source of confusion for my-self and others with the `--check-cfg` syntax.

> The confusing part here is that an empty `values()` currently means the same as `values(none())`, i.e. an expected list of values with the _none_ variant (as in `#[cfg(name)]` where the value is none) instead of meaning an empty set.

Before the new `cfg()` syntax, defining the _none_ variant was only possible under certain circumstances, so in https://github.com/rust-lang/rust/pull/111068 I decided to make `values()` to mean the _none_ variant, but it is no longer necessary since https://github.com/rust-lang/rust/pull/119473 which introduced the `none()` syntax.

A simplified representation of the proposed "de-specialization" would be:

| Syntax                                  | List/set of expected values |
|-----------------------------------------|-----------------------------|
| `cfg(name)`/`cfg(name, values(none()))` | `&[None]`                   |
| `cfg(name, values())`                   | `&[]`                       |

Note that I have my-self made the mistake of using an empty `values()` as meaning empty set, see https://github.com/rust-lang/cargo/pull/13011.

`@rustbot` label +F-check-cfg
r? `@petrochenkov`
cc `@epage`
This commit is contained in:
bors 2024-01-17 14:01:05 +00:00
commit c58a5da7d4
10 changed files with 89 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
compiler/rustc_interface/src/interface.rs
View File

@ -218,7 +218,9 @@ macro_rules! error {
}
}
if values.is_empty() && !values_any_specified && !any_specified {
if !values_specified && !any_specified {
// `cfg(name)` is equivalent to `cfg(name, values(none()))` so add
// an implicit `none()`
values.insert(None);
} else if !values.is_empty() && values_any_specified {
error!(

11
compiler/rustc_lint/src/context/diagnostics.rs
View File

@ -354,6 +354,15 @@ pub(super) fn builtin(
Applicability::MaybeIncorrect,
);
}
} else {
db.note(format!("no expected values for `{name}`"));
let sp = if let Some((_value, value_span)) = value {
name_span.to(value_span)
} else {
name_span
};
db.span_suggestion(sp, "remove the condition", "", Applicability::MaybeIncorrect);
}
// We don't want to suggest adding values to well known names
@ -373,6 +382,8 @@ pub(super) fn builtin(
if name == sym::feature {
if let Some((value, _value_span)) = value {
db.help(format!("consider adding `{value}` as a feature in `Cargo.toml`"));
} else {
db.help("consider defining some features in `Cargo.toml`");
}
} else if !is_cfg_a_well_know_name {
db.help(format!("consider using a Cargo feature instead or adding `println!(\"cargo:rustc-check-cfg={inst}\");` to the top of a `build.rs`"));

25
src/doc/unstable-book/src/compiler-flags/check-cfg.md
View File

@ -44,15 +44,20 @@ To enable checking of values, but to provide an *none*/empty set of expected val
```bash
rustc --check-cfg 'cfg(name)'
rustc --check-cfg 'cfg(name, values())'
rustc --check-cfg 'cfg(name, values(none()))'
```
To enable checking of name but not values (i.e. unknown expected values), use this form:
To enable checking of name but not values, use one of these forms:
```bash
rustc --check-cfg 'cfg(name, values(any()))'
```
- 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:
@ -114,18 +119,14 @@ as argument to `--check-cfg`.
This table describe the equivalence of a `--cfg` argument to a `--check-cfg` argument.
| `--cfg` | `--check-cfg` |
|-----------------------------|----------------------------------------------------------|
|-------------------------------|------------------------------------------------------------|
| *nothing* | *nothing* or `--check-cfg=cfg()` (to enable the checking) |
| `--cfg foo` | `--check-cfg=cfg(foo), --check-cfg=cfg(foo, values())` or `--check-cfg=cfg(foo, values(none()))` |
| `--cfg foo` | `--check-cfg=cfg(foo)` or `--check-cfg=cfg(foo, values(none()))` |
| `--cfg foo=""` | `--check-cfg=cfg(foo, values(""))` |
| `--cfg foo="bar"` | `--check-cfg=cfg(foo, values("bar"))` |
| `--cfg foo="1" --cfg foo="2"` | `--check-cfg=cfg(foo, values("1", "2"))` |
| `--cfg foo="1" --cfg bar="2"` | `--check-cfg=cfg(foo, values("1")) --check-cfg=cfg(bar, values("2"))` |
| `--cfg foo --cfg foo="bar"` | `--check-cfg=cfg(foo) --check-cfg=cfg(foo, values("bar"))` |
NOTE: There is (currently) no way to express that a condition name is expected but no (!= none)
values are expected. Passing an empty `values()` means *(none)* in the sense of `#[cfg(foo)]`
with no value. Users are expected to NOT pass a `--check-cfg` with that condition name.
| `--cfg foo --cfg foo="bar"` | `--check-cfg=cfg(foo, values(none(), "bar"))` |
### Example: Cargo-like `feature` example

20
tests/ui/check-cfg/cargo-feature.none.stderr
View File

@ -1,34 +1,36 @@
warning: unexpected `cfg` condition name: `feature`
--> $DIR/cargo-feature.rs:13:7
warning: unexpected `cfg` condition value: `serde`
--> $DIR/cargo-feature.rs:14:7
|
LL | #[cfg(feature = "serde")]
| ^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^ help: remove the condition
|
= help: consider defining some features in `Cargo.toml`
= note: no expected values for `feature`
= help: consider adding `serde` as a feature in `Cargo.toml`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default
warning: unexpected `cfg` condition name: `feature`
warning: unexpected `cfg` condition value: (none)
--> $DIR/cargo-feature.rs:18:7
|
LL | #[cfg(feature)]
| ^^^^^^^
| ^^^^^^^ help: remove the condition
|
= note: no expected values for `feature`
= help: consider defining some features in `Cargo.toml`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
warning: unexpected `cfg` condition name: `tokio_unstable`
--> $DIR/cargo-feature.rs:23:7
--> $DIR/cargo-feature.rs:22:7
|
LL | #[cfg(tokio_unstable)]
| ^^^^^^^^^^^^^^
|
= help: expected names are: `debug_assertions`, `doc`, `doctest`, `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: expected names are: `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(tokio_unstable)");` to the top of a `build.rs`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
warning: unexpected `cfg` condition name: `CONFIG_NVME`
--> $DIR/cargo-feature.rs:27:7
--> $DIR/cargo-feature.rs:26:7
|
LL | #[cfg(CONFIG_NVME = "m")]
| ^^^^^^^^^^^^^^^^^

9
tests/ui/check-cfg/cargo-feature.rs
View File

@ -5,19 +5,18 @@
// check-pass
// revisions: some none
// rustc-env:CARGO=/usr/bin/cargo
// compile-flags: --check-cfg=cfg() -Z unstable-options
// compile-flags: -Z unstable-options
// [none]compile-flags: --check-cfg=cfg(feature,values())
// [some]compile-flags: --check-cfg=cfg(feature,values("bitcode"))
// [some]compile-flags: --check-cfg=cfg(CONFIG_NVME,values("y"))
// [none]error-pattern:Cargo.toml
#[cfg(feature = "serde")]
//[none]~^ WARNING unexpected `cfg` condition name
//[some]~^^ WARNING unexpected `cfg` condition value
//~^ WARNING unexpected `cfg` condition value
fn ser() {}
#[cfg(feature)]
//[none]~^ WARNING unexpected `cfg` condition name
//[some]~^^ WARNING unexpected `cfg` condition value
//~^ WARNING unexpected `cfg` condition value
fn feat() {}
#[cfg(tokio_unstable)]

7
tests/ui/check-cfg/cargo-feature.some.stderr
View File

@ -1,5 +1,5 @@
warning: unexpected `cfg` condition value: `serde`
--> $DIR/cargo-feature.rs:13:7
--> $DIR/cargo-feature.rs:14:7
|
LL | #[cfg(feature = "serde")]
| ^^^^^^^^^^^^^^^^^
@ -16,10 +16,11 @@ LL | #[cfg(feature)]
| ^^^^^^^- help: specify a config value: `= "bitcode"`
|
= note: expected values for `feature` are: `bitcode`
= help: consider defining some features in `Cargo.toml`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
warning: unexpected `cfg` condition name: `tokio_unstable`
--> $DIR/cargo-feature.rs:23:7
--> $DIR/cargo-feature.rs:22:7
|
LL | #[cfg(tokio_unstable)]
| ^^^^^^^^^^^^^^
@ -29,7 +30,7 @@ LL | #[cfg(tokio_unstable)]
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration
warning: unexpected `cfg` condition value: `m`
--> $DIR/cargo-feature.rs:27:7
--> $DIR/cargo-feature.rs:26:7
|
LL | #[cfg(CONFIG_NVME = "m")]
| ^^^^^^^^^^^^^^---

4
tests/ui/check-cfg/concat-values.rs
View File

@ -1,6 +1,7 @@
// check-pass
// compile-flags: -Z unstable-options
// compile-flags: --check-cfg=cfg(my_cfg,values("foo")) --check-cfg=cfg(my_cfg,values("bar"))
// compile-flags: --check-cfg=cfg(my_cfg,values())
#[cfg(my_cfg)]
//~^ WARNING unexpected `cfg` condition value
@ -10,4 +11,7 @@ fn my_cfg() {}
//~^ WARNING unexpected `cfg` condition value
fn my_cfg() {}
#[cfg(any(my_cfg = "foo", my_cfg = "bar"))]
fn foo_and_bar() {}
fn main() {}

4
tests/ui/check-cfg/concat-values.stderr
View File

@ -1,5 +1,5 @@
warning: unexpected `cfg` condition value: (none)
--> $DIR/concat-values.rs:5:7
--> $DIR/concat-values.rs:6:7
|
LL | #[cfg(my_cfg)]
| ^^^^^^
@ -10,7 +10,7 @@ LL | #[cfg(my_cfg)]
= note: `#[warn(unexpected_cfgs)]` on by default
warning: unexpected `cfg` condition value: `unk`
--> $DIR/concat-values.rs:9:7
--> $DIR/concat-values.rs:10:7
|
LL | #[cfg(my_cfg = "unk")]
| ^^^^^^^^^^^^^^

14
tests/ui/check-cfg/empty-values.rs Normal file
View File

@ -0,0 +1,14 @@
// Check that we detect unexpected value when none are allowed
//
// check-pass
// compile-flags: --check-cfg=cfg(foo,values()) -Zunstable-options
#[cfg(foo = "foo")]
//~^ WARNING unexpected `cfg` condition value
fn do_foo() {}
#[cfg(foo)]
//~^ WARNING unexpected `cfg` condition value
fn do_foo() {}
fn main() {}

23
tests/ui/check-cfg/empty-values.stderr Normal file
View File

@ -0,0 +1,23 @@
warning: unexpected `cfg` condition value: `foo`
--> $DIR/empty-values.rs:6:7
|
LL | #[cfg(foo = "foo")]
| ^^^^^^^^^^^ help: remove the condition
|
= note: no expected values for `foo`
= help: to expect this configuration use `--check-cfg=cfg(foo, values("foo"))`
= 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: (none)
--> $DIR/empty-values.rs:10:7
|
LL | #[cfg(foo)]
| ^^^ help: remove the condition
|
= note: no expected values for `foo`
= help: to expect this configuration use `--check-cfg=cfg(foo)`
= 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