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

Urgau · 2023-05-01T16:46:01Z

Old proposition (before the MCP)

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.

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.

This PR is the first part of the implementation of MCP636 - Simplify and improve explicitness of the check-cfg syntax.

New `cfg` form

It introduces the new cfg form 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 #111068 but is reviewable (last two commits)!~~

Resolve rust-lang/compiler-team#636

r? @petrochenkov

rustbot · 2023-05-01T16:46:11Z

Error: Parsing relabel command in comment failed: ...'-on-author' | error: a label delta at >| ' (for the'...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

petrochenkov · 2023-05-02T09:47:28Z

Blocked on #111068.

petrochenkov · 2023-05-10T15:28:03Z

How can exhaustive(values) be a single flag?
Isn't value checking enabled and disabled per-name?
I.e. we can check values for name foo, and not check values for name bar.
Or does it only affect cfgs built into the compiler?

petrochenkov · 2023-05-10T15:32:01Z

Does --check-cfg exhaustive(names, values) basically mean --check-cfg builtin(names, values)?
What does --check-cfg exhaustive(values) mean without names?

UPD: Looks like --check-cfg exhaustive(values) does mean --check-cfg builtin(values), but --check-cfg exhaustive(names) does not.

petrochenkov · 2023-05-10T15:57:35Z

With this new design we cannot specify a name as known, while keeping its list of values unknown, right?

Previously names(foo) didn't restrict the set of values for foo, but exhaustive(names) & configure(foo) does restrict it to "no values".

Urgau · 2023-05-10T15:58:04Z

How can exhaustive(values) be a single flag? Isn't value checking enabled and disabled per-name? I.e. we can check values for name foo, and not check values for name bar. Or does it only affect cfgs built into the compiler?

exhaustive(values) works in the same way as values(), it adds (nearly) all the builtin cfgs with their expected values into the expected list. That means that all the builtin cfgs will lint if there is an unexpected value and that's it.
It doesn't change anything for non-builtin cfgs.

Remember that we can only lint on values for which we have an entry for, if we don't have one then it's not a value problem but a name problem and this is gated under exhaustive(names).

Does --check-cfg exhaustive(names, values) basically mean --check-cfg builtin(names, values)?

Not exactly, for values yes, it's an append of the builtin values. But for names it's not only appending the list of builtin names but also enabling the checking of all names; otherwise we wouldn't be able to check names since we would always trigger on builtin names.

What does --check-cfg exhaustive(values) mean without names?

Enable checking of the builtin values without triggering on potentially unexpected cfg.

Urgau · 2023-05-10T16:04:57Z

With this new design we cannot specify a name as known, while keeping its list of values unknown, right?

Yes, this is the only "regression" of the new design, but I've yet to found a example where this would be useful. The only place where it is useful is in the compiler for feature and target_feature where getting the expected values is non trivial.

If a consumer of rustc doesn't know the expected values of a config, how would a actual user know which one to use? Or even how does the consumer of rustc know which one to set via --cfg?

Previously names(foo) didn't restrict the set of values for foo, but exhaustive(names) & configure(foo) does restrict it to "no values".

Btw, no need for exhaustive(names) here, doing configure(foo) no matter if exhaustive(names) is set, set the expected values to none.

@rustbot ready

petrochenkov · 2023-05-10T16:28:09Z

So what we really need to cover all cases without regressions (without any syntax beautifications) is:

--check-cfg builtin-values - will be commonly used if we expect full checking to be typically used.
--check-cfg custom-values(noval, oneval, "val1", twovals, "val1", "val2") - will be commonly used if we expect full checking to be typically used.
--check-cfg exhaustive-names(foo, bar) - doesn't put value restrictions on foo and bar, --check-cfg exhaustive-names() without arguments will be commonly used if we expect full checking to be typically used, the form with arguments - not so commonly.

petrochenkov · 2023-05-10T16:33:03Z

In that case we probably need some short syntax for the commonly used combination --check-cfg builtin-values --check-cfg exhaustive-names() --check-cfg custom-values(noval, oneval, "val1", twovals, "val1", "val2") + some additional rarely used options for opting out of exhaustiveness and adding names without value requirements.

Urgau · 2023-05-10T16:46:19Z

So what we really need to cover all cases without regressions (without any syntax beautifications) is:

--check-cfg builtin-values - will be commonly used if we expect full checking to be typically used.

--check-cfg custom-values(noval, oneval, "val1", twovals, "val1", "val2") - will be commonly used if we expect full checking to be typically used.

Agree for these two.

--check-cfg exhaustive-names(foo, bar) - doesn't put value restrictions on foo and bar,

As I said, I don't think this is useful for users, as users knows all the possible values. Otherwise they couldn't possible use cfgs in any meaningful way.

--check-cfg exhaustive-names() without arguments will be commonly used if we expect full checking to be typically used, the form with arguments - not so commonly.

👍 (but note that there isn't currently any plan to enable this by default in cargo)

In that case we probably need some short syntax for the commonly used combination --check-cfg builtin-values --check-cfg exhaustive-names() --check-cfg custom-values(noval, oneval, "val1", twovals, "val1", "val2") + some additional rarely used options for opting out of exhaustiveness and adding names without value requirements.

If cargo were to enable all things, it would be with my syntax exhaustive(names, values)&configure(feature, ...) set by cargo + users defined (but cannot really do anything about them).

petrochenkov · 2023-10-17T15:26:49Z

@bors r+

bors · 2023-10-17T15:26:51Z

📌 Commit fc78d78 has been approved by petrochenkov

It is now in the queue for this repository.

WaffleLapkin · 2023-10-17T17:55:46Z

@Urgau is the syntax in the OP still up to date? (I don't immediately get what the syntax means, specifically the values(...) so I'm asking to make sure) And is this feature gated? Or is it insta-stable?

Urgau · 2023-10-17T18:35:55Z

Urgau is the syntax in the OP still up to date? (I don't immediately get what the syntax means, specifically the values(...) so I'm asking to make sure)

Yes, the PR description, the commit message and the MCP are all up to date.
Regarding values(...) (I assume you mean in cfg(...) and not the old syntax), it's a designator to separate the names previously given and the values associated with them. In other words, it's like a key=value pair, where both the names (key) and the values are de-duplicated.

For more information, I would highly suggest reading the check-cfg section of the rustc unstable book.

And is this feature gated? Or is it insta-stable?

It's a pre-existing unstable feature, so yes it's gated.

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#111072 (Add new simpler and more explicit syntax for check-cfg) - rust-lang#116717 (Special case iterator chain checks for suggestion) - rust-lang#116719 (Add MonoItems and Instance to stable_mir) - rust-lang#116787 (Implement an internal lint encouraging use of `Span::eq_ctxt`) - rust-lang#116827 (Make `handle_options` public again.) r? `@ghost` `@rustbot` modify labels: rollup

WaffleLapkin · 2023-10-17T18:58:54Z

(Sorry for commenting on details after MCP was accepted and PR approved...) For me it looks weird that values() is it's own thing. From the syntax it looks like an element just like feature names, not a thing on a different level. I'd expect something more like name = ["value", "value2"] or name("value", "value2"), rather than name, values("value", value2), again, because name and values here don't live on the same level, they are not elements of the same list.

Also the documentation seems to have a configure in an example, which looks like a mistake?

Anyway, thanks for answering my questions.

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`

57: Pull upstream master 2023 10 18 r=pietroalbini a=Veykril * rust-lang/rust#116505 * rust-lang/rust#116840 * rust-lang/rust#116767 * rust-lang/rust#116855 * rust-lang/rust#116827 * rust-lang/rust#116787 * rust-lang/rust#116719 * rust-lang/rust#116717 * rust-lang/rust#111072 * rust-lang/rust#116844 * rust-lang/rust#115577 * rust-lang/rust#116756 * rust-lang/rust#116518 Co-authored-by: Urgau <urgau@numericable.fr> Co-authored-by: Esteban Küber <esteban@kuber.com.ar> Co-authored-by: Deadbeef <ent3rm4n@gmail.com> Co-authored-by: Ralf Jung <post@ralfj.de> Co-authored-by: Camille GILLOT <gillot.camille@gmail.com> Co-authored-by: Celina G. Val <celinval@amazon.com> Co-authored-by: Nicholas Nethercote <n.nethercote@gmail.com> Co-authored-by: Arthur Lafrance <lafrancearthur@gmail.com> Co-authored-by: Nikolay Arhipov <n@arhipov.net> Co-authored-by: Nikita Popov <npopov@redhat.com> Co-authored-by: bors <bors@rust-lang.org>

Urgau · 2023-10-18T15:52:33Z

values(...) being it's "own" thing is on purpose, cf. https://hackmd.io/@wesleywiser/rJlumbfep:

I think we should specifically denote the values. Perhaps cfg(feature, values(foo, bar)). No values would be spelled cfg(feature) or cfg(feature, values()). This makes it easer to extend the constraints we can express in the future (for example cfg(target_pointer_width, one_of(16, 32, 64))).

btw, if you wish to continue discussing it let's do that on Zulip

Also the documentation seems to have a configure in an example, which looks like a mistake?

Yes, missed-it.

Adjust `-Zcheck-cfg` for new rustc syntax and behavior rust-lang/rust#111072 introduced a new syntax for `rustc` `--check-cfg` argument. This PR adjust cargo `-Zcheck-cfg` for new that new syntax and behavior. This PR removes all the `-Zcheck-cfg` options (`features`, `names`, `values`, `output`), as they don't make much sense now since with the new `rustc` behavior: `features`, `names` and `values` are all combine together and the `output` option was only here because the other were. Now the new behavior from cargo is to always pass one `--check-cfg` argument to rustc for the `feature`s which implicitly enables well known names and values.

…k-cfg, r=<try> Prepare the `bootstrap` tool for the new check-cfg syntax This PR prepare the `bootstrap` tool for the [new check-cfg syntax](rust-lang#111072) as well as the according [changes to Cargo](rust-lang/cargo#12845). Note that while the new syntax can technically available on stage > 2, we actually cannot use it since we need a cargo version that supports the new syntax which won't happen until the next beta bump (if I understand everything correctly). r? bootstrap

…k-cfg, r=Kobzol Prepare the `bootstrap` tool for the new check-cfg syntax This PR prepare the `bootstrap` tool for the [new check-cfg syntax](rust-lang#111072) as well as the according [changes to Cargo](rust-lang/cargo#12845). ~~Note that while the new syntax can technically available on stage > 2, we actually cannot use it since we need a cargo version that supports the new syntax which won't happen until the next beta bump (if I understand everything correctly).~~ r? bootstrap

…Kobzol Prepare the `bootstrap` tool for the new check-cfg syntax This PR prepare the `bootstrap` tool for the [new check-cfg syntax](rust-lang/rust#111072) as well as the according [changes to Cargo](rust-lang/cargo#12845). ~~Note that while the new syntax can technically available on stage > 2, we actually cannot use it since we need a cargo version that supports the new syntax which won't happen until the next beta bump (if I understand everything correctly).~~ r? bootstrap

…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

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

…Kobzol Prepare the `bootstrap` tool for the new check-cfg syntax This PR prepare the `bootstrap` tool for the [new check-cfg syntax](rust-lang/rust#111072) as well as the according [changes to Cargo](rust-lang/cargo#12845). ~~Note that while the new syntax can technically available on stage > 2, we actually cannot use it since we need a cargo version that supports the new syntax which won't happen until the next beta bump (if I understand everything correctly).~~ r? bootstrap

rustbot assigned petrochenkov May 1, 2023

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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels May 1, 2023

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 1, 2023

This comment has been minimized.

Sign in to view

Urgau force-pushed the check-cfg-new-syntax branch from a01a326 to 3b54cec Compare May 1, 2023 17:02

This comment has been minimized.

Sign in to view

Urgau force-pushed the check-cfg-new-syntax branch from 3b54cec to e072419 Compare May 1, 2023 17:12

This comment was marked as resolved.

Sign in to view

Urgau force-pushed the check-cfg-new-syntax branch from e072419 to 6846f33 Compare May 5, 2023 19:54

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels May 5, 2023

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 10, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 10, 2023

rustbot removed S-waiting-on-MCP Status: PR has a compiler MCP and is waiting for the compiler MCP to complete. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Oct 17, 2023

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 Oct 17, 2023

matthiaskrgr mentioned this pull request Oct 17, 2023

Rollup of 5 pull requests #116855

Merged

bors merged commit ce40742 into rust-lang:master Oct 17, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Oct 17, 2023

Veykril mentioned this pull request Oct 18, 2023

Pull upstream master 2023 10 18 ferrocene/ferrocene#57

Merged

Urgau mentioned this pull request Oct 18, 2023

Adjust -Zcheck-cfg for new rustc syntax and behavior rust-lang/cargo#12845

Merged

Urgau mentioned this pull request Oct 20, 2023

Prepare the bootstrap tool for the new check-cfg syntax #116983

Merged

Urgau mentioned this pull request Oct 23, 2023

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

Closed

3 tasks

Urgau mentioned this pull request Nov 16, 2023

Remove deprecated --check-cfg syntax #117981

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Urgau commented May 1, 2023 •

edited

Loading

rustbot commented May 1, 2023

This comment has been minimized.

This comment has been minimized.

petrochenkov commented May 2, 2023

This comment was marked as resolved.

petrochenkov commented May 10, 2023

petrochenkov commented May 10, 2023 •

edited

Loading

petrochenkov commented May 10, 2023

Urgau commented May 10, 2023

Urgau commented May 10, 2023 •

edited

Loading

petrochenkov commented May 10, 2023

petrochenkov commented May 10, 2023

Urgau commented May 10, 2023

petrochenkov commented Oct 17, 2023

bors commented Oct 17, 2023

WaffleLapkin commented Oct 17, 2023 •

edited

Loading

Urgau commented Oct 17, 2023 •

edited

Loading

WaffleLapkin commented Oct 17, 2023

Urgau commented Oct 18, 2023

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

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

Conversation

Urgau commented May 1, 2023 • edited Loading

New cfg form

Default built-in names and values

rustbot commented May 1, 2023

This comment has been minimized.

This comment has been minimized.

petrochenkov commented May 2, 2023

This comment was marked as resolved.

petrochenkov commented May 10, 2023

petrochenkov commented May 10, 2023 • edited Loading

petrochenkov commented May 10, 2023

Urgau commented May 10, 2023

Urgau commented May 10, 2023 • edited Loading

petrochenkov commented May 10, 2023

petrochenkov commented May 10, 2023

Urgau commented May 10, 2023

petrochenkov commented Oct 17, 2023

bors commented Oct 17, 2023

WaffleLapkin commented Oct 17, 2023 • edited Loading

Urgau commented Oct 17, 2023 • edited Loading

WaffleLapkin commented Oct 17, 2023

Urgau commented Oct 18, 2023

Urgau commented May 1, 2023 •

edited

Loading

New `cfg` form

petrochenkov commented May 10, 2023 •

edited

Loading

Urgau commented May 10, 2023 •

edited

Loading

WaffleLapkin commented Oct 17, 2023 •

edited

Loading

Urgau commented Oct 17, 2023 •

edited

Loading