Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Feb 11, 2025

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 by rustc_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

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 11, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 11, 2025

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

These commits modify compiler targets.
(See the Target Tier Policy.)

@rust-log-analyzer

This comment has been minimized.

@jieyouxu

This comment was marked as resolved.

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 11, 2025
@madsmtm
Copy link
Contributor Author

madsmtm commented Feb 11, 2025

since it looks like some of the design questions haven't quite settled yet

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 rustc_os_version_min in the meantime?

@Urgau
Copy link
Member

Urgau commented Feb 11, 2025

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.
@madsmtm
Copy link
Contributor Author

madsmtm commented Feb 11, 2025

T-lang experiment

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)

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-18 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
#21 exporting to docker image format
#21 sending tarball 28.2s done
#21 DONE 37.2s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-18]
debug: `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` configured.
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-18', '--enable-llvm-link-shared', '--set', 'rust.randomize-layout=true', '--set', 'rust.thin-lto-import-instr-limit=10', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-18/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.randomize-layout := True
configure: rust.thin-lto-import-instr-limit := 10
---
  Downloaded boml v0.3.1
   Compiling boml v0.3.1
   Compiling y v0.1.0 (/checkout/compiler/rustc_codegen_gcc/build_system)
    Finished `release` profile [optimized] target(s) in 3.97s
     Running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --mini-tests --std-tests`
Using system GCC
warning: target feature `x87` must be enabled to ensure that the ABI of the current target can be implemented correctly
  |
  = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>
---
##[group]Testing stage2 book unstable-book (x86_64-unknown-linux-gnu)
STDOUT:

running 1 test
test /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md - The_tracking_issue_for_this_feature_is__::Examples (line 41) ... FAILED
failures:


---- /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md - The_tracking_issue_for_this_feature_is__::Examples (line 41) stdout ----
error: expected one of `!`, `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `,`
##[error]  --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:55:30
   |
16 |         fn preadv(libc::c_int, *const libc::iovec, libc::c_int, libc::off64_t) -> libc::ssize_t;
   |
   = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
help: explicitly ignore the parameter name
   |
   |
16 |         fn preadv(_: libc::c_int, *const libc::iovec, libc::c_int, libc::off64_t) -> libc::ssize_t;

error: expected parameter name, found `*`
error: expected parameter name, found `*`
##[error]  --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:55:32
   |
16 |         fn preadv(libc::c_int, *const libc::iovec, libc::c_int, libc::off64_t) -> libc::ssize_t;


error: expected one of `!`, `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `,`
##[error]  --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:55:63
   |
16 |         fn preadv(libc::c_int, *const libc::iovec, libc::c_int, libc::off64_t) -> libc::ssize_t;
   |
   = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
help: explicitly ignore the parameter name
   |
   |
16 |         fn preadv(libc::c_int, *const libc::iovec, _: libc::c_int, libc::off64_t) -> libc::ssize_t;


error: expected one of `!`, `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)`
##[error]  --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:55:78
   |
16 |         fn preadv(libc::c_int, *const libc::iovec, libc::c_int, libc::off64_t) -> libc::ssize_t;
   |
   = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
help: explicitly ignore the parameter name
   |
   |
16 |         fn preadv(libc::c_int, *const libc::iovec, libc::c_int, _: libc::off64_t) -> libc::ssize_t;

error: unexpected token: `...`
error: unexpected token: `...`
##[error]  --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:72:12
   |
33 |     preadv(...) // Use preadv, it's available
   |
help: use `..` for an exclusive range
   |
   |
33 -     preadv(...) // Use preadv, it's available
33 +     preadv(..) // Use preadv, it's available
help: or `..=` for an inclusive range
   |
   |
33 -     preadv(...) // Use preadv, it's available
33 +     preadv(..=) // Use preadv, it's available

error[E0586]: inclusive range with no end
error[E0586]: inclusive range with no end
##[error]  --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:72:12
   |
33 |     preadv(...) // Use preadv, it's available
   |
   |
   = note: inclusive ranges must be bounded at the end (`..=b` or `a..=b`)
   |
   |
33 -     preadv(...) // Use preadv, it's available
33 +     preadv(..) // Use preadv, it's available

warning: version is set unnecessarily low, the minimum supported by Rust on this platform is 1.0
warning: version is set unnecessarily low, the minimum supported by Rust on this platform is 1.0
##[warning]  --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:51:32
   |
12 |     os_version_min("visionos", "1.0")
   |     ---------------------------^^^^^- help: use `target_os` instead: `target_os = "visionos"`
warning: version is set unnecessarily low, the minimum supported by Rust on this platform is 1.0
warning: version is set unnecessarily low, the minimum supported by Rust on this platform is 1.0
##[warning]  --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:67:32
   |
28 |     os_version_min("visionos", "1.0")
   |     ---------------------------^^^^^- help: use `target_os` instead: `target_os = "visionos"`
error[E0432]: unresolved import `libc`
error[E0432]: unresolved import `libc`
##[error] --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:43:5
4 | use libc;
  |     ^^^^ no `libc` in the root

error: cannot find macro `weak` in this scope
error: cannot find macro `weak` in this scope
##[error]  --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:69:1
   |
30 | weak!(fn preadv(libc::c_int, *const libc::iovec, libc::c_int, libc::off64_t) -> libc::ssize_t);


error[E0425]: cannot find value `preadv` in this scope
##[error]  --> /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md:71:23
   |
32 | if let Some(preadv) = preadv {

error: aborting due to 9 previous errors; 2 warnings emitted

Some errors have detailed explanations: E0425, E0432, E0586.
Some errors have detailed explanations: E0425, E0432, E0586.
For more information about an error, try `rustc --explain E0425`.
Couldn't compile the test.

failures:
    /checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md - The_tracking_issue_for_this_feature_is__::Examples (line 41)
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.06s




Command CFG_RELEASE_CHANNEL="nightly" RUSTC_BOOTSTRAP="1" RUSTC_STAGE="2" RUSTC_SYSROOT="/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" RUSTDOC_LIBDIR="/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" RUSTDOC_REAL="/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" RUST_TEST_THREADS="16" "/checkout/obj/build/bootstrap/debug/rustdoc" "-Wrustdoc::invalid_codeblock_attributes" "-Dwarnings" "-Znormalize-docs" "-Z" "unstable-options" "--test" "/checkout/src/doc/unstable-book/src/language-features/cfg-os-version-min.md" "--test-args" "" (failure_mode=DelayFail) has failed. Rerun with -v to see more details.
  local time: Tue Feb 11 15:15:55 UTC 2025
  network time: Tue, 11 Feb 2025 15:15:56 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@jieyouxu
Copy link
Member

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 rustc_os_version_min in the meantime?

Probably worth trying this as a T-lang experiment yeah

@jieyouxu jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Feb 11, 2025
Copy link
Member

@SparrowLii SparrowLii left a 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

@rustbot rustbot assigned jieyouxu and unassigned SparrowLii Feb 12, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 12, 2025
…=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.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 12, 2025
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.
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Feb 13, 2025
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.
@bors
Copy link
Contributor

bors commented Feb 15, 2025

☔ The latest upstream changes (presumably #137046) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants