Conversation
weiznich
left a comment
There was a problem hiding this comment.
Overall I linke the idea, but I have some questions and remarks
(Obvious disclaimer: I'm not associated with any team, that's just my personal opinion, so feel free to ignore that)
RFCs are the place for community feedback. Insight from future feature users that aren't team members is just as (if not more) valuable, and always welcome in all discussion areas ❤️ |
This reverts commit 11de2c7. It had some in-progress changes I didn't mean to commit.
Co-authored-by: Mads Marquart <mads@marquart.dk>
Co-authored-by: Mads Marquart <mads@marquart.dk>
Co-authored-by: Ed Page <eopage@gmail.com>
|
|
||
| The version string parsing can be extended to support pre-release identifiers (`-beta`, `-nightly`), though this adds complexity to the comparison logic. RFC 3857 discusses this possibility for [generic versions](https://github.com/rust-lang/rfcs/blob/4551bbd827eb84fc6673ac0204506321274ea839/text/3857-cfg-version.md#relaxing-version) as well as for the [language itself](https://github.com/rust-lang/rfcs/blob/4551bbd827eb84fc6673ac0204506321274ea839/text/3857-cfg-version.md#pre-release). | ||
|
|
||
| Like 3857, this RFC does not support prereleases. It assumes prereleases info will be stripped from version numbers. |
There was a problem hiding this comment.
This also sounds like normative text that is put in a non-normative section of the RFC
|
|
||
| The version string parsing can be extended to support pre-release identifiers (`-beta`, `-nightly`), though this adds complexity to the comparison logic. RFC 3857 discusses this possibility for [generic versions](https://github.com/rust-lang/rfcs/blob/4551bbd827eb84fc6673ac0204506321274ea839/text/3857-cfg-version.md#relaxing-version) as well as for the [language itself](https://github.com/rust-lang/rfcs/blob/4551bbd827eb84fc6673ac0204506321274ea839/text/3857-cfg-version.md#pre-release). | ||
|
|
||
| Like 3857, this RFC does not support prereleases. It assumes prereleases info will be stripped from version numbers. |
There was a problem hiding this comment.
Except, as of last T-lang discussion, pre-release was not going to be stripped from Rust but instead it was going to be the previous version number. Feels incongruous to say one thing and design around it, and then we do something different.
| ### Systems that fall outside this RFC | ||
|
|
||
| - **Python packages**: Python's package versioning system is defined in [PEP 440](https://peps.python.org/pep-0440/) to have five segments, only one of which (the release segment) is directly supported in this RFC. This level of flexibility seems to exist because it was designed to cover the many diverging versioning schemes used by Python packages at the time it was introduced. (Thanks to Ed Page [on github](https://github.com/rust-lang/rfcs/pull/3905#discussion_r2677156303).) | ||
|
|
||
| > Public version identifiers are separated into up to five segments: | ||
| > | ||
| > - Epoch segment: `N!` | ||
| > - Release segment: `N(.N)*` | ||
| > - Pre-release segment: `{a|b|rc}N` | ||
| > - Post-release segment: `.postN` | ||
| > - Development release segment: `.devN` |
There was a problem hiding this comment.
This is unfortunate that this falls outside of this RFC as there is a large use of Rust in Python and it looks to only be increasing.
RFC 3857 overlooked epochs and ordering issues between decv and pre-release but did discuss a strategy for post-releases: https://github.com/rust-lang/rfcs/blob/4551bbd827eb84fc6673ac0204506321274ea839/text/3857-cfg-version.md#cfg_target_version
| - **Namespaced `cfg`s:** We can group Rust-specific `cfg`s under a `rust::` namespace, e.g., `#[cfg(rust::version >= "1.85")]`. This RFC intentionally keeps `rust_version` at the top level to simplify the initial implementation and stabilization, but namespacing can be explored in the future to better organize the growing number of built-in `cfg`s. | ||
| - **Macro that evaluates to a cfg value:** We can add a `cfg_value!()` macro for single-valued configs that evaluates to its value. For versions this could evaluate to a string literal. | ||
| - **Short-circuiting `cfg` predicates:** Change `any` and `all` predicates to short-circuit instead of evaluating all their arguments. This would make introducing new predicates and comparison operators much easier. | ||
| - **Const eval in `cfg`:** In the future `cfg` can be expressed entirely in terms of const eval, with a simple macro desugaring for cfg-specific predicates into proper Rust expressions. This RFC is compatible with that vision; see [this demo](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9207bb80756b64ea99f51f54c3369f0d) of how the desugaring can work. |
There was a problem hiding this comment.
Cargo has hitched itself to Rust's cfg system and this would be incompatible with Cargo
|
|
||
| A `cfg` identifier is of type `version` if: | ||
| * It is one of the built-in identifiers `rust_version` or `rust_edition`. | ||
| * It is declared with `--check-cfg 'cfg(name, version)'` and is passed to the compiler with the special syntax `--cfg 'name=version("...")'`. |
There was a problem hiding this comment.
Using --check-cfg is entirely optional, I assume the --check-cfg is meant to be optional and only the --cfg 'name=version("...")' is actually required.
| * It is declared with `--check-cfg 'cfg(name, version)'` and is passed to the compiler with the special syntax `--cfg 'name=version("...")'`. | |
| * It is declared with the special syntax `--cfg 'name=version("...")'` and if using `--check-cfg` is declared appropriately with `--check-cfg 'cfg(name, version())'`. |
| * **`--check-cfg`**: To inform the compiler that a `cfg` is expected to be a version, and to enable linting, use: | ||
| ```sh | ||
| --check-cfg 'cfg(my_app_version, version())' | ||
| ``` | ||
|
|
||
| This will accept any version value, but lint when the option is used in a non-version comparison (note that this is an error if the option actually has a version-typed value). Accepting any version is a more sensible default for `version()`, which doesn't have the equivalent of `values(none())`. |
There was a problem hiding this comment.
It would be unfortunate to define version() are meaning "Unknown expected versions", while for values() is means "No expected values".
The difference being that with values() the compiler lint on every value of name (useful to say that a cfg name does exist at some point but not currently, like Cargo feature cfg, when no features are currently defined).
We use a different syntax values(any()) to tell the compiler to never lint on any value of name.
I would therefor really prefer to be consistent with values(...) and support:
version(any())for "Unknown expected versions"version()for "No expected versions"
| * **`--check-cfg`**: To inform the compiler that a `cfg` is expected to be a version, and to enable linting, use: | |
| ```sh | |
| --check-cfg 'cfg(my_app_version, version())' | |
| ``` | |
| This will accept any version value, but lint when the option is used in a non-version comparison (note that this is an error if the option actually has a version-typed value). Accepting any version is a more sensible default for `version()`, which doesn't have the equivalent of `values(none())`. | |
| * **`--check-cfg`**: To inform the compiler that a `cfg` is expected to be a version, and to enable linting, use: | |
| ```sh | |
| --check-cfg 'cfg(my_app_version, version(any()))' | |
| ``` | |
| This will accept any version value, but lint when the option is used in a non-version comparison (note that this is an error if the option actually has a version-typed value). |
This RFC proposes a general mechanism for version-based conditional compilation called "typed cfgs".
Summary
This RFC proposes "typed
cfgs", a new form of conditional compilation predicate that understands types. Initially, this RFC proposes to add support for version-typedcfgs, allowing for ergonomic version comparisons against the language version supported by the compiler. This would be exposed through two new built-incfgnames:rust_version, which can be compared against a language version literal, e.g.,#[cfg(rust_version >= "1.85")].rust_edition, which can be compared against an edition literal, e.g.,#[cfg(rust_edition >= "2024")].This design solves a long-standing problem of conditionally compiling code for different Rust versions without requiring build scripts or forcing libraries to increase their Minimum Supported Rust Version (MSRV). It also replaces the
cfg(version(..))part of RFC 2523.History
There have been several previous attempts to solve the problem of conditional compilation by Rust version.
#[cfg(version(..))]. However, it left the syntax as an open question. Attempts to stabilize it were blocked because the new syntax would be a hard error on older compilers, defeating the goal of supporting older MSRVs.#[cfg(version_since(rust, "1.95"))]for Rust-version conditional compilation #3857 proposed#[cfg(version_since(rust, ...))], which solved the MSRV problem and generalized the mechanism beyond Rust versions while defining the interaction with command line flags like--check-cfg.This RFC takes the lessons from both previous attempts. It proposes a path to the ergonomic
rust_version >= "..."syntax that was preferred during language team discussions, while providing a clear MSRV-compatibility story from day one.The RFC also incorporates use cases from the
cfg_target_versionRFC (#3750), which proposed a way to compare against the version of the target platform's SDK (e.g.,#[cfg(target_version(macos >= "10.15"))]). Version-typed cfgs provide a path to supporting these comparions.Finally, it takes cues from previous discussions around mutually exclusive features and a
cfg_value!()macro, and lays out a path toward more single-valued config types that could support these features.Rendered