Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Simplify and improve explicitness of the check-cfg syntax #636

Closed
1 of 3 tasks
Urgau opened this issue May 30, 2023 · 3 comments · Fixed by rust-lang/rust#111072
Closed
1 of 3 tasks

Simplify and improve explicitness of the check-cfg syntax #636

Urgau opened this issue May 30, 2023 · 3 comments · Fixed by rust-lang/rust#111072
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team

Comments

@Urgau
Copy link
Member

Urgau commented May 30, 2023

Proposal

This proposal is a spawned of work started in this PR. Current documentation for check-cfg can be found in the unstable book.

Old proposal

This proposal wants to adds a new simpler and more explicit syntax for check-cfg. It would consist of two new form [detailed doc]:

  1. --check-cfg exhaustive(...) enables some exhaustive checking
  2. --check-cfg configure(...) enables checking the values within list-valued conditions.

The exhaustive(...) form

The exhaustive(...) form tell the compiler if it should be exhaustive about it's checking.

It currently has two exhaustiveness:

  • names: permist the compiler to check every config name by adding it's list of well-known names
  • values: permist the compiler to check it's well-known cfg values by adding it's list of well-known values

The configure(...) form

The configure(...) form enables checking the values within list-valued conditions. It has this form:

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

To enable checking of values, but to provide an empty set of valid values, use this form:

rustc --check-cfg `configure(name1, ..., nameN)`

The --check-cfg configure(...) 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.

Why?

The preview forms names(...) and values(...) have implicit meaning that are not strait-forward. In particular values(foo)&values(bar) and names(foo, bar) are not equivalent which has created some confusions.

Also the names() and values() form are not clear either and again created some confusions where peoples believed that values()&values(foo) could be reduced to just values(foo).

To fix that the two new forms are made to be explicit and simpler. See the table of correspondence:

  • names() -> exhaustive(names)
  • values() -> exhaustive(values)
  • names(foo) -> exhaustive(names)&configure(foo)
  • values(foo) -> configure(foo)
  • values(feat, "foo", "bar") -> configure(feat, "foo", "bar")
  • values(foo)&values(bar) -> configure(foo, bar)
  • names()&values()&values(my_cfg) -> exhaustive(names, values)&configure(my_cfg)

The two previous forms would be deprecated and will would be removed once cargo and beta rustc have the necessary support.

Open questions

  • Naming of configure (comment):

    --check-cfg configure(...) also sounds tautological, cfg already means "configure", values was a better name for this semantic, IMO.

    Could be kept as values(...) without too much difficulties, having a different name seemed clearer that it was different.

  • Naming of exhaustive:
    Too different from names to re-use the same name.

This proposal is based on the previous proposal above and the summary work done by @wesleywiser in https://hackmd.io/@wesleywiser/rJlumbfep.

This MCP proposes to:

  • remove the names() and values() form of --check-cfg (with at least one release of deprecation)
  • replace those two forms by new unified form one called cfg() (to remove most of un-intuitiveness of the previous syntax)

The cfg(...) form

The cfg(...) form enables checking the values within list-valued conditions. It has this form:

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

To enable checking of values, but to provide an empty set of expected values, use these forms:

rustc --check-cfg 'cfg(name1, ..., nameN)'
rustc --check-cfg 'cfg(name1, ..., nameN, values())'

To enable checking of name but not values (i.e. unknown expected values), use this form:

rustc --check-cfg 'cfg(name1, ..., nameN, values(any()))'

To disable checking of name, use this form:

rustc --check-cfg 'cfg(any())'

The --check-cfg cfg(...) option can be repeated, both for the same condition name and for different names. If it is repeated for the same condition name, then the sets of values for that condition are merged together.

Built-in names and values

  • Built-in values checking would always be activated as long as a --check-cfg argument is present
  • Built-in names checking would always be activated as long as a --check-cfg argument is present unless if any cfg(any()) arg is passed
  • If one want to enable values and names checking without having any cfg to declare, one can use an empty cfg()

Why?

The preview forms names(...) and values(...) have implicit meaning that are not strait-forward. In particular values(foo)&values(bar) and names(foo, bar) are not equivalent which has created some confusions.

Also the names() and values() form are not clear either and again created some confusions where peoples believed that values()&values(foo) could be reduced to just values(foo).

To fix that the two new forms are made to be explicit and simpler. See the table of correspondence:

  • names() -> enable-by-default by any use of --check-cfg useless given cfg(any())
  • values() -> enable-by-default by any use of --check-cfg
  • names(foo) -> cfg(foo, any())
  • values(foo) -> cfg(foo)
  • values(feat, "foo", "bar") -> cfg(feat, values("foo", "bar"))
  • values(foo)&values(bar) -> cfg(foo, bar)
  • names()&values()&values(my_cfg) -> cfg(my_cfg)

The two previous forms would be deprecated and will would be removed once cargo and beta rustc have the necessary support.

Open questions

  • Naming of cfg form (comment):

    --check=cfg(...). I had this thought as well but was struggling to think of other things it could be extended to. It also seemed like users might think rustc --check ... is equivalent to cargo check. Need to think about this more...

    rustdoc already has a --check argument making --check=cfg(...) impossible

Mentors or Reviewers

@petrochenkov for reviewing and my-self for the implementation work

Process

The main points of the Major Change Process are as follows:

  • File an issue describing the proposal.
  • A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@Urgau Urgau added major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team labels May 30, 2023
@rustbot
Copy link
Collaborator

rustbot commented May 30, 2023

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

@rustbot rustbot added the to-announce Announce this issue on triage meeting label May 30, 2023
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jun 14, 2023
@wesleywiser
Copy link
Member

@rustbot second

@rustbot rustbot added the final-comment-period The FCP has started, most (if not all) team members are in agreement label Oct 5, 2023
@apiraino
Copy link
Contributor

@rustbot label -final-comment-period +major-change-accepted

@rustbot rustbot added major-change-accepted A major change proposal that was accepted to-announce Announce this issue on triage meeting and removed final-comment-period The FCP has started, most (if not all) team members are in agreement labels Oct 17, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 17, 2023
…ochenkov

Add new simpler and more explicit syntax for check-cfg

<details>
<summary>
Old proposition (before the MCP)
</summary>

This PR adds a new simpler and more explicit syntax for check-cfg. It consist of two new form:
 - `exhaustive(names, values)`
 - `configure(name, "value1", "value2", ... "valueN")`

The preview forms `names(...)` and `values(...)` have implicit meaning that are not strait-forward. In particular `values(foo)`&`values(bar)` and `names(foo, bar)` are not equivalent which has created [some confusions](rust-lang#98080).

Also the `names()` and `values()` form are not clear either and again created some confusions where peoples believed that `values()`&`values(foo)` could be reduced to just `values(foo)`.

To fix that the two new forms are made to be explicit and simpler. See the table of correspondence:
  - `names()` -> `exhaustive(names)`
  - `values()` -> `exhaustive(values)`
  - `names(foo)` -> `exhaustive(names)`&`configure(foo)`
  - `values(foo)` -> `configure(foo)`
  - `values(feat, "foo", "bar")` -> `configure(feat, "foo", "bar")`
  - `values(foo)`&`values(bar)` -> `configure(foo, bar)`
  - `names()`&`values()`&`values(my_cfg)` -> `exhaustive(names, values)`&`configure(my_cfg)`

Another benefits of the new syntax is that it allow for further options (like conditional checking for --cfg, currently always on) without syntax change.

The two previous forms are deprecated and will be removed once cargo and beta rustc have the necessary support.

</details>

This PR is the first part of the implementation of [MCP636 - Simplify and improve explicitness of the check-cfg syntax](rust-lang/compiler-team#636).

## New `cfg` form

It introduces the new [`cfg` form](rust-lang/compiler-team#636) and deprecate the other two:
```
rustc --check-cfg 'cfg(name1, ..., nameN, values("value1", "value2", ... "valueN"))'
```

## Default built-in names and values

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

 - Built-in values checking would always be activated as long as a `--check-cfg` argument is present
 - Built-in names checking would always be activated as long as a `--check-cfg` argument is present **unless** if any `cfg(any())` arg is passed

~~**Note: depends on rust-lang#111068 but is reviewable (last two commits)!**~~

Resolve rust-lang/compiler-team#636

r? `@petrochenkov`
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 17, 2023
Rollup merge of rust-lang#111072 - Urgau:check-cfg-new-syntax, r=petrochenkov

Add new simpler and more explicit syntax for check-cfg

<details>
<summary>
Old proposition (before the MCP)
</summary>

This PR adds a new simpler and more explicit syntax for check-cfg. It consist of two new form:
 - `exhaustive(names, values)`
 - `configure(name, "value1", "value2", ... "valueN")`

The preview forms `names(...)` and `values(...)` have implicit meaning that are not strait-forward. In particular `values(foo)`&`values(bar)` and `names(foo, bar)` are not equivalent which has created [some confusions](rust-lang#98080).

Also the `names()` and `values()` form are not clear either and again created some confusions where peoples believed that `values()`&`values(foo)` could be reduced to just `values(foo)`.

To fix that the two new forms are made to be explicit and simpler. See the table of correspondence:
  - `names()` -> `exhaustive(names)`
  - `values()` -> `exhaustive(values)`
  - `names(foo)` -> `exhaustive(names)`&`configure(foo)`
  - `values(foo)` -> `configure(foo)`
  - `values(feat, "foo", "bar")` -> `configure(feat, "foo", "bar")`
  - `values(foo)`&`values(bar)` -> `configure(foo, bar)`
  - `names()`&`values()`&`values(my_cfg)` -> `exhaustive(names, values)`&`configure(my_cfg)`

Another benefits of the new syntax is that it allow for further options (like conditional checking for --cfg, currently always on) without syntax change.

The two previous forms are deprecated and will be removed once cargo and beta rustc have the necessary support.

</details>

This PR is the first part of the implementation of [MCP636 - Simplify and improve explicitness of the check-cfg syntax](rust-lang/compiler-team#636).

## New `cfg` form

It introduces the new [`cfg` form](rust-lang/compiler-team#636) and deprecate the other two:
```
rustc --check-cfg 'cfg(name1, ..., nameN, values("value1", "value2", ... "valueN"))'
```

## Default built-in names and values

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

 - Built-in values checking would always be activated as long as a `--check-cfg` argument is present
 - Built-in names checking would always be activated as long as a `--check-cfg` argument is present **unless** if any `cfg(any())` arg is passed

~~**Note: depends on rust-lang#111068 but is reviewable (last two commits)!**~~

Resolve rust-lang/compiler-team#636

r? `@petrochenkov`
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Oct 19, 2023
bors added a commit to rust-lang/libc that referenced this issue Oct 27, 2023
Use new check-cfg syntax in newer nightly

[MCP636 - Simplify and improve explicitness of the check-cfg syntax](rust-lang/compiler-team#636) introduced a new syntax for check-cfg.

This PR adjust the `build.rs` code to use the new syntax on `rustc >= 75`.
The old syntax is still used on older version since on rust-lang/rust we need to be compatible with both for now.
fee1-dead added a commit to fee1-dead-contrib/rust that referenced this issue Dec 5, 2023
…syntax, r=b-naber

Remove deprecated `--check-cfg` syntax

This PR removes the deprecated `--check-cfg` `names(...)` and `values(...)` syntax.

Follow up to rust-lang#111072
Part of rust-lang/compiler-team#636

r? compiler
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 6, 2023
…syntax, r=b-naber

Remove deprecated `--check-cfg` syntax

This PR removes the deprecated `--check-cfg` `names(...)` and `values(...)` syntax.

Follow up to rust-lang#111072
Part of rust-lang/compiler-team#636

r? compiler
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 7, 2023
Rollup merge of rust-lang#117981 - Urgau:check-cfg-remove-deprecated-syntax, r=b-naber

Remove deprecated `--check-cfg` syntax

This PR removes the deprecated `--check-cfg` `names(...)` and `values(...)` syntax.

Follow up to rust-lang#111072
Part of rust-lang/compiler-team#636

r? compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants