-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Implement cfg_os_version_min
#136867
base: master
Are you sure you want to change the base?
Implement cfg_os_version_min
#136867
Conversation
Also convert OSVersion into a proper struct for better type-safety.
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred in compiler/rustc_attr_parsing These commits modify compiler targets. |
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
Hmm, I was hoping to resolve those in future PRs, since I believe this feature is useful to the standard library today (or rather yesterday), regardless of future semantics and syntax. Would it help if I renamed it to |
You should ask T-lang to approve this PR as an "T-lang experiment", that way it could implemented in the compiler even without an FCP on the RFC. |
Based on in-progress RFC: rust-lang/rfcs#3750. Only implemented for Apple platforms for now, but written in a way that should be easily expandable to include other platforms.
6125af0
to
bb55488
Compare
Right, that's what it's called, couldn't remember, thanks! (filed #136871 so that I'll be able to find it in the future) |
The job Click to see the possible cause of the failure (guessed by this bot)
|
Probably worth trying this as a T-lang experiment yeah |
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.
Thanks for your work! But I know little about this feature so r? compiler
…=scottmcm dev-guide: Link to `t-lang` procedures for new features I was confused in rust-lang#136867, because while I did remember that such a procedure existed, but I couldn't seem to find it in the dev guide.
Rollup merge of rust-lang#136871 - madsmtm:link-to-lang-procedures, r=scottmcm dev-guide: Link to `t-lang` procedures for new features I was confused in rust-lang#136867, because while I did remember that such a procedure existed, but I couldn't seem to find it in the dev guide.
dev-guide: Link to `t-lang` procedures for new features I was confused in rust-lang/rust#136867, because while I did remember that such a procedure existed, but I couldn't seem to find it in the dev guide.
☔ The latest upstream changes (presumably #137046) made this pull request unmergeable. Please resolve the merge conflicts. |
Implement the
cfg_os_version_min
feature that is being RFC'd in rust-lang/rfcs#3750, tracking issue #136866. This implementation only handles Apple targets, but it should be fairly easy to add support for other targets after this.The RFC is not finalized, and as such things may change (especially the syntax). Regardless, I think it makes sense to start experimenting with it; even if the feature is ultimately rejected, it is necessary for the standard library (an example is that it would allow us to ship #122408 without a dangerous fallback).
Implementation-wise, the first two commits are refactoring required to move some Apple deployment target support code out of
rustc_codegen_ssa
and into a place where it could be accessed byrustc_attr_parsing
. The third commit implements the feature.Using this in the standard library is done in a draft PR.
CC @ChrisDenton @BlackHoleFox
@rustbot label O-apple
r? rust-lang/compiler