-
Notifications
You must be signed in to change notification settings - Fork 13k
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
bootstrap: compactify --check-cfg arguments #98080
Conversation
The current code creates more --check-cfg arguments than needed, especially one argument per bare name. To save arguments, we now collect the name specific arguments into one. Before, the code this commit changes would have generated these arguments for a typical compiler crate: '--check-cfg=values(bootstrap)' '--check-cfg=values(parallel_compiler)' '--check-cfg=values(no_btreemap_remove_entry)' '--check-cfg=values(crossbeam_loom)' '--check-cfg=values(span_locations)' Now it generates: '--check-cfg=names(bootstrap,parallel_compiler,no_btreemap_remove_entry,crossbeam_loom,span_locations)'
(rust-highfive has picked a reviewer for you, use r? to override) |
cc @Urgau |
Those are not equivalent, This change would allow someone to mistakenly write |
Oh I see, I've thought they were. Thanks for your explanation. I wonder if there could be a |
Maybe, but I don't think it's a big issue, most projects that will need to manually set Also as we can see here the syntax implication is already not simple and I fear that adding another thing will only make things worse not better. |
I see both names() and values() in the diff. What's the state of this PR? Is it possible to use a shorter syntax while keeping the same meaning or should it just be closed? |
(I agree with @Urgau that making the invocation slightly shorter doesn't seem worth making the syntax more complicated.) |
Yeah let's close this. I'm still not happy about it, and the difference is only that |
…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`
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`
The current code creates more --check-cfg arguments than needed,
especially one argument per bare name. To save arguments,
we now collect the name specific arguments into one.
Before, the code this commit changes would have generated
these arguments for a typical compiler crate:
Now it generates: