Skip to content

Add explicit none() value variant in check-cfg #119473

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 13, 2024
Merged

Add explicit none() value variant in check-cfg #119473

merged 1 commit into from
Jan 13, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Dec 31, 2023 •
edited
Loading

This PR adds an explicit none value variant in check-cfg values: values(none()).

Currently the only way to define the none variant is with an empty values() which means that if someone has a cfg that takes none and strings they need to use two invocations: --check-cfg=cfg(foo) --check-cfg=cfg(foo, values("bar")).
Which would now be --check-cfg=cfg(foo, values(none(),"bar")), this is simpler and easier to understand.

--check-cfg=cfg(foo), --check-cfg=cfg(foo, values()) and --check-cfg=cfg(foo, values(none())) would be equivalent.

Another motivation for doing this is to make empty values() actually means no-values, but this is orthogonal to this PR and adding none() is sufficient in it-self.

@rustbot label +F-check-cfg
r? @petrochenkov

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. F-check-cfg --check-cfg labels Dec 31, 2023
@bors

This comment was marked as resolved.

# to view
@Urgau Urgau force-pushed the check-cfg-explicit-none branch from 5f6f562 to 15078c2 Compare January 9, 2024 18:12
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 12, 2024

📌 Commit 15078c2 has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 12, 2024
@petrochenkov
Copy link
Contributor

Another motivation for doing this is to make empty values() actually means no-values

I don't understand this line though, what is the difference between "none" and "no-values"?

@bors
Copy link
Collaborator

bors commented Jan 13, 2024

⌛ Testing commit 15078c2 with merge 7585c62...

@bors
Copy link
Collaborator

bors commented Jan 13, 2024

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 7585c62 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 13, 2024
@bors bors merged commit 7585c62 into rust-lang:master Jan 13, 2024
@rustbot rustbot added this to the 1.77.0 milestone Jan 13, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (7585c62): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 2.1% [2.1%, 2.1%] 1
Regressions ❌
(secondary)		 2.6% [2.6%, 2.6%] 2
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 2.1% [2.1%, 2.1%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 668.566s -> 668.237s (-0.05%)
Artifact size: 308.16 MiB -> 308.17 MiB (0.01%)

@Urgau Urgau mentioned this pull request Jan 13, 2024
Merged
@Urgau
Copy link
Member Author

Urgau commented Jan 13, 2024

Another motivation for doing this is to make empty values() actually means no-values

I don't understand this line though, what is the difference between "none" and "no-values"?

I have opened #119930 to explain and tackle this; but to simplify, 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). I propose to remove this confusing meaning by making an empty values() actually mean a empty set of expected values.
See more details and explanation in the PR.

@petrochenkov
Copy link
Contributor

@Urgau
I see, I initially thought that the "empty set of expected values" would be meaningless (you just don't add the name to the set of expected names in that case), but #119930 shows why it may make sense.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 17, 2024
@bors
Auto merge of rust-lang#119930 - Urgau:check-cfg-empty-values-means-e…
c58a5da 
…mpty, r=petrochenkov

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](https://github.com/rust-lang/cargo/blob/92395d90106b3b61bcb68bcf2069052c93771764/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 rust-lang#111068 I decided to make `values()` to mean the _none_ variant, but it is no longer necessary since rust-lang#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 rust-lang/cargo#13011.

`@rustbot` label +F-check-cfg
r? `@petrochenkov`
cc `@epage`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jan 18, 2024
@bors
Auto merge of #119930 - Urgau:check-cfg-empty-values-means-empty, r=p…
ae6972c 
…etrochenkov

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](https://github.com/rust-lang/cargo/blob/92395d90106b3b61bcb68bcf2069052c93771764/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 rust-lang/rust#111068 I decided to make `values()` to mean the _none_ variant, but it is no longer necessary since rust-lang/rust#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 rust-lang/cargo#13011.

`@rustbot` label +F-check-cfg
r? `@petrochenkov`
cc `@epage`
This was referenced Jan 18, 2024
Closed
Closed
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
@bors
Auto merge of #119930 - Urgau:check-cfg-empty-values-means-empty, r=p…
44ce579 
…etrochenkov

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](https://github.com/rust-lang/cargo/blob/92395d90106b3b61bcb68bcf2069052c93771764/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 rust-lang/rust#111068 I decided to make `values()` to mean the _none_ variant, but it is no longer necessary since rust-lang/rust#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 rust-lang/cargo#13011.

`@rustbot` label +F-check-cfg
r? `@petrochenkov`
cc `@epage`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
@bors
Auto merge of #119930 - Urgau:check-cfg-empty-values-means-empty, r=p…
a8a0df1 
…etrochenkov

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](https://github.com/rust-lang/cargo/blob/92395d90106b3b61bcb68bcf2069052c93771764/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 rust-lang/rust#111068 I decided to make `values()` to mean the _none_ variant, but it is no longer necessary since rust-lang/rust#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 rust-lang/cargo#13011.

`@rustbot` label +F-check-cfg
r? `@petrochenkov`
cc `@epage`
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@petrochenkov petrochenkov

Labels
F-check-cfg --check-cfg merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.77.0
Development

Successfully merging this pull request may close these issues.
5 participants
@Urgau @bors @petrochenkov @rust-timer @rustbot