Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add min_$TARGET_API_version cfg predicates #3036
Add min_$TARGET_API_version cfg predicates #3036
Changes from all commits
a19a888
b0f9400
fcec804
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
How would a library set its minimum version? I don't see anything to that effect in this section nor in the reference-level explanation.
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.
This is actually an unanswered question - thanks for bringing it up. The RFC mentions storing this information in the crate metadata but gives no mechanism for doing so. I'm not aware of any existing mechanism that we could use for such a thing. The closest that comes to mind is
#![no_std]
, but having a#![min_windows_build_version]
seems like overkill.If we switch to a dedicated syntax like
#[min_version(...)]
we could conceivably just store the smallest of those used. Perhaps this is another point in favor of the dedicated syntax.Originally the proposal was to not have libraries store any information about their minimum supported target. They would simply conditionally compile based on what was passed as the
cfg
flag through rustc. This was changed at some point, but I'm not sure that was a good idea.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.
The
min_
prefix in the proposed cfg labels and the>=
operator seem duplicate each other in the meaning. Since>=
is a new syntax (as is acknowledged later) and would most likely need its own rfc, I think we want to avoid>=
in this rfc.(besides, if we push for
>=
, what would then the behaviour of#[cfg(min_windows_build_version="6.2.9200")]
be?)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.
The semantics of the
min_*_version
is that marked code should compile when the set minimum supported build version is at least equal to that version. i.e., if code is marked with#[cfg(min_windows_build_version >= "windows7")]
, it should compile if themin_windows_build_version
is set to "windows10". It has been argued that a plain "=" implies that the code would only compile when themin_windows_build_version
matches the version provided exactly.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.
The Zulip thread linked up top goes back and forth on
>=
a couple of times. It might be worth revisiting some of the arguments there to help move the discussion forward and save circling around the same points.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.
Nit:
#[cfg(min_windows_build_version >= "6.2.9200"]
is missing a closing paren.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.
What would be the expected behaviour in case of invocation such as
(both when no
cfg
s withmin_windows_build_version
and when there are some present in the crate code)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.
Why does the compiler need to know about each value of the API version string? It seems like the compiler should just forward the version string to each crate as it builds them.
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 drawback that comes to mind is that it is possible for users to specify arbitrary
foo="x"
cfgs today, and so this change (and any future addition for other platforms) as proposed would not be backwards compatible in a strict interpretation of compatibility.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 would strongly prefer this. Firstly this avoids the can of worms related to backwards compatibility.
As a secondhand benefit something like this could also pave the road to different matching predicates within the version string, much like what we have in cargo. So for instance, crates use semver and so a variety of operators that make sense for semver can be used (rather than just
>=
):If, out there, there's a target that utilizes semantic versioning for itself or its components (I think
freebsd
is a possible close match here), then utilizing the familiar semver matching algorithms would also make sense for it.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.
Specifying a generic mechanism for targets to provide their own versioning schemes, rather than concrete
cfg
s also sounds like a more productive direction to me. Then the fate of this RFC wouldn't need to linger on us being comfortable with all sorts of versioning schemes proposed for the various targets. Once an RFC with a generic mechanism is merged, we could spend our time on figuring out out the high value versioning cfgs (windows, macos) sooner via other channels (PR FCPs, MCPs etc) even before we know what to do about linux, android or bsds.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.
Amen. Better to have a single syntax for all operating systems, rather than defining a separate variable name for each.
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 question, what does versioning on linux based OSs mean? Should we switch on the kernel version, libc version, distro version or something else.
Eg on android, the relevent version is the API version, not the kernel version. Linux distros are unique (afaik) for being the only os where the kernel and other parts of the API are versioned seperatly.
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.
Yeah it almost sounds like there should be different version keys or so that a target can have.
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 must admit I was only thinking about the libc version. What are some examples of changes to the kernel that would not bubble through to changes in libc which the user would like to conditionally compile code on?
What is meant by
target_api_version
is the platform's API that it exposes for end applications.