-
Notifications
You must be signed in to change notification settings - Fork 1.6k
RFC: #[cfg(since(rust, "1.95"))]
for Rust-version conditional compilation
#3857
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
base: master
Are you sure you want to change the base?
Conversation
RE my assumptions around owning teams for this decision:
CC @rust-lang/compiler , @rust-lang/clippy , @rust-lang/cargo |
CC @Urgau as this touches a lot on |
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
People may be using `--cfg rust` already and would be broken by this change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least with my initial search, I did not see any public uses
This reverts commit e51caf2.
I forgot where I landed on how to handle them
|
||
This does not help with probes for specific implementations of nightly features which still requires having a `build.rs`. | ||
|
||
Libraries could having ticking time bombs that accidentally break or have undesired behavior for some future Rust version that can't be found until we hit that version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you mean because of them using #[cfg(since(rust, "1.$BIG_NUMBER"))]
or similar "point a loaded firearm directly at my lower limb" gestures? They sort of already can do that, with the classic example being mem::transmute
of std
entities with entirely-unspecified layouts, and then us changing it in a future version. But yes, strictly speaking it does introduce a new vector, and one we will inevitably hit in time rather than merely potentially.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this is something that could be warn-by-default. Or at least warn-by-default in clippy. The warning would only be an issue when a workspace is worked on by rust versions older than their highest version cfg.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have any way of detecting this. No toolchain can flag "too new" version requirements, because it's reasonable to detect things newer than that toolchain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I would assume this problem already exists with rustversion
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I expect it will be very common to have a crate that is developed and tested primarily with an old rustc version and then have since
used against a much later one so that new features can also be used selectively, so warning against this issue will mostly interfere with what may be the most popular pattern for using the feature.
*but* maybe that would just be the start to natively supporting more types in `cfg`, | ||
like integers which are of interest to embedded folks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels somewhat unclear to me why we would start this journey by supporting this new "version literal" type in cfg
only and not in the language? As opposed to supporting integers first, which are supported in the language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that I'm saying doing so is a potential alternative; I am not proposed we do. If we did so, the reason would be "because this RFC came first".
Co-authored-by: bjorn3 <[email protected]>
text/3857-cfg-version.md
Outdated
- Contort the version into SemVer | ||
- This can run into problems either with having more precision (e.g. `120.0.1.10` while SemVer only allows `X.Y.Z`) or post-release versions (e.g. `1.2.0.post1` which a SemVer predicate would treat as a pre-release). | ||
- Add an optional third field for specifying the version format (e.g. `#[cfg(since(windows, "10.0.10240", <policy-name>)]`) | ||
- Make `--check-cfg` load-bearing by having the version policy name be specified in the `--check-cfg` predicate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One depressingly verbose option, but one that will work without modification to the proposal, is to attach an additional "epoch version" via cfg
, e.g.
#[cfg(
all(
since(thing, "1.0.0"), since(thing_epoch, "2")
)
)]
mod something {}
#[cfg(
all(
since(thing, "1.1.0"), not(since(thing_epoch, "2"))
)
)]
mod something {}
Obviously, since(thing, "1.0.0")
would be satisfied by "1.1.0"
in ordering without the all
including an additional "version epoch".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure how much we need to explore this as its a future possibility. The main question is whether we paint ourselves into a corner for target_version
.
Co-authored-by: Ralf Jung <[email protected]>
If I'm reading the RFC right, this attribute is very interesting in that it generalizes to other external dependencies too. In PyO3, we have to deal with a ton of variation for different Python versions. Our current solution is to emit I wonder if this would immediately also follow for cargo dependencies too. E.g. |
Currently, the RFC requires SemVer versions. I am exploring the future possibility that will relax that and seeing what impact that has on core proposal
Cargo providing dependency versions to the compiler is listed as a future possibility. |
This reverts commit 7139923.
text/3857-cfg-version.md
Outdated
``` | ||
|
||
When evaluating `since`, | ||
1. If the string literal does not conform to the syntax from `<major>` to `<major>.<minor>.<patch>-<pre-release>` where the first three fields must be integers, this will evaluate to `false`.<br> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An invalid SemVer syntax for the string literal should probably be a compile time error, that would allow for future extension without being breaking changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another reason for making it a error would be to avoid not(since(foo, "<invalid>"))
always returning true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Urgau <[email protected]>
Tracking issue (
#[cfg(version(..))]
Rendered
Summary
Allow Rust-version conditional compilation by adding a built-in
--cfg rust=<version>
and a minimum-version#[cfg]
predicate.Say this was added before 1.70, you could do:
This supersedes the
cfg_version
subset of RFC 2523.