Skip to content

Make #[cfg(version)] respect RUSTC_OVERRIDE_VERSION_STRING #141413

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 25, 2025

Conversation

est31
Copy link
Member

@est31 est31 commented May 22, 2025

The #[cfg(version(...))] feature is currently under-tested. Part of it is the difficulty that it is hard to write a test that never changes, while the version of the Rust compiler indeed does change.

PR #81468 added the first and so far only test of #[cfg(version(...))]'s functionality (there is one other test for the syntax, that also acts as feature gate). But that test uses a proc macro that parses the version: the text of the test doesn't contain the actual #[cfg(version(...))].

This PR makes #[cfg(version(...))] respect RUSTC_OVERRIDE_VERSION_STRING, added by PR #124339, allowing us to virtually pin the rustc version and write tests from all directions against some specific version.

The PR also adds a functional test of #[cfg(version(...))] that leverages RUSTC_OVERRIDE_VERSION_STRING.

Pulled out of #141137.

Tracking issue: #64796

@rustbot
Copy link
Collaborator

rustbot commented May 22, 2025

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the A-attributes Area: Attributes (`#[…]`, `#![…]`) label May 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 22, 2025

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_attr_data_structures

cc @jdonszelmann

@rustbot rustbot added 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 May 22, 2025
@est31 est31 mentioned this pull request May 22, 2025
3 tasks
@jieyouxu
Copy link
Member

r? jieyouxu

@rustbot rustbot assigned jieyouxu and unassigned BoxyUwU May 23, 2025
@jieyouxu jieyouxu added the F-cfg_version `#![feature(cfg_version)]` label May 23, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Two things:

  1. A naming nit.
  2. Can you squash the commits into one? The second test commit doesn't need to be its own commit IMO.

@jieyouxu
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2025
@jieyouxu
Copy link
Member

(Also, feel free to r? me for cfg_version PRs since I was already looking at the previous stabilization PR)

@est31 est31 force-pushed the cfg_version_env_var branch from f294432 to fdaca73 Compare May 23, 2025 12:12
@est31
Copy link
Member Author

est31 commented May 23, 2025

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 23, 2025
@jieyouxu
Copy link
Member

jieyouxu commented May 23, 2025

Thanks, feel free to r=me after the static naming nit.

EDIT: actually one sec

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 23, 2025

📌 Commit 8616237 has been approved by jieyouxu

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented May 23, 2025

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 23, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 23, 2025
Make #[cfg(version)] respect RUSTC_OVERRIDE_VERSION_STRING

The `#[cfg(version(...))]` feature is currently under-tested. Part of it is the difficulty that it is hard to write a test that never changes, while the version of the Rust compiler indeed *does* change.

PR rust-lang#81468 added the first and so far only test of `#[cfg(version(...))]`'s functionality (there is one other test for the *syntax*, that also acts as feature gate). But that test uses a proc macro that parses the version: the text of the test doesn't contain the actual `#[cfg(version(...))]`.

This PR makes `#[cfg(version(...))]` respect `RUSTC_OVERRIDE_VERSION_STRING`, added by PR rust-lang#124339, allowing us to virtually pin the rustc version and write tests from all directions against some specific version.

The PR also adds a functional test of `#[cfg(version(...))]` that leverages `RUSTC_OVERRIDE_VERSION_STRING`.

Pulled out of rust-lang#141137.

Tracking issue: rust-lang#64796
bors added a commit that referenced this pull request May 24, 2025
Rollup of 8 pull requests

Successful merges:

 - #140790 (Enable xray support for Mac)
 - #141405 (GetUserProfileDirectoryW is now documented to always store the size)
 - #141413 (Make #[cfg(version)] respect RUSTC_OVERRIDE_VERSION_STRING)
 - #141427 (Disable `triagebot`'s `glacier` handler)
 - #141429 (Dont walk into unsafe binders when emiting error for non-structural type match)
 - #141438 (Do not try to confirm non-dyn compatible method)
 - #141444 (Improve CONTRIBUTING.md grammar and clarity)
 - #141446 (Add 2nd Solaris target maintainer)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 24, 2025
Make #[cfg(version)] respect RUSTC_OVERRIDE_VERSION_STRING

The `#[cfg(version(...))]` feature is currently under-tested. Part of it is the difficulty that it is hard to write a test that never changes, while the version of the Rust compiler indeed *does* change.

PR rust-lang#81468 added the first and so far only test of `#[cfg(version(...))]`'s functionality (there is one other test for the *syntax*, that also acts as feature gate). But that test uses a proc macro that parses the version: the text of the test doesn't contain the actual `#[cfg(version(...))]`.

This PR makes `#[cfg(version(...))]` respect `RUSTC_OVERRIDE_VERSION_STRING`, added by PR rust-lang#124339, allowing us to virtually pin the rustc version and write tests from all directions against some specific version.

The PR also adds a functional test of `#[cfg(version(...))]` that leverages `RUSTC_OVERRIDE_VERSION_STRING`.

Pulled out of rust-lang#141137.

Tracking issue: rust-lang#64796
@matthiaskrgr
Copy link
Member

@bors r-
#141482 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 24, 2025
@est31 est31 force-pushed the cfg_version_env_var branch from 8616237 to f3245e4 Compare May 24, 2025 21:54
@est31 est31 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 24, 2025
@est31
Copy link
Member Author

est31 commented May 24, 2025

fixed the build failure after a rebase.

@jieyouxu
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented May 25, 2025

📌 Commit f3245e4 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 25, 2025
bors added a commit that referenced this pull request May 25, 2025
Rollup of 6 pull requests

Successful merges:

 - #141413 (Make #[cfg(version)] respect RUSTC_OVERRIDE_VERSION_STRING)
 - #141443 (make teach_help message for cast-before-pass-to-variadic more precise)
 - #141508 (bootstrap: clippy: set TESTNAME based on given paths)
 - #141512 (Avoid extra path trimming in method not found error)
 - #141530 (Added unstable feature doc comments to unstable book)
 - #141541 (Random nits)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b0ae228 into rust-lang:master May 25, 2025
6 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 25, 2025
rust-timer added a commit that referenced this pull request May 25, 2025
Rollup merge of #141413 - est31:cfg_version_env_var, r=jieyouxu

Make #[cfg(version)] respect RUSTC_OVERRIDE_VERSION_STRING

The `#[cfg(version(...))]` feature is currently under-tested. Part of it is the difficulty that it is hard to write a test that never changes, while the version of the Rust compiler indeed *does* change.

PR #81468 added the first and so far only test of `#[cfg(version(...))]`'s functionality (there is one other test for the *syntax*, that also acts as feature gate). But that test uses a proc macro that parses the version: the text of the test doesn't contain the actual `#[cfg(version(...))]`.

This PR makes `#[cfg(version(...))]` respect `RUSTC_OVERRIDE_VERSION_STRING`, added by PR #124339, allowing us to virtually pin the rustc version and write tests from all directions against some specific version.

The PR also adds a functional test of `#[cfg(version(...))]` that leverages `RUSTC_OVERRIDE_VERSION_STRING`.

Pulled out of #141137.

Tracking issue: #64796
jieyouxu added a commit to jieyouxu/rust that referenced this pull request May 26, 2025
Pull out dedicated `cfg_version` syntax test from feature gate test

Tracking issue: rust-lang#64796.
Closes rust-lang#141452, as a follow-up to rust-lang#141413 (comment) (point 3 of that is probably too pedantic).

The feature gate test was dual-purposing causing feature gate errors to distract from syntax exercises.

`@rustbot` label +F-cfg_version
r? `@est31`
compiler-errors added a commit to compiler-errors/rust that referenced this pull request May 27, 2025
Pull out dedicated `cfg_version` syntax test from feature gate test

Tracking issue: rust-lang#64796.
Closes rust-lang#141452, as a follow-up to rust-lang#141413 (comment) (point 3 of that is probably too pedantic).

The feature gate test was dual-purposing causing feature gate errors to distract from syntax exercises.

``@rustbot`` label +F-cfg_version
r? ``@est31``
rust-timer added a commit that referenced this pull request May 27, 2025
Rollup merge of #141552 - jieyouxu:cfg-version-tests, r=est31

Pull out dedicated `cfg_version` syntax test from feature gate test

Tracking issue: #64796.
Closes #141452, as a follow-up to #141413 (comment) (point 3 of that is probably too pedantic).

The feature gate test was dual-purposing causing feature gate errors to distract from syntax exercises.

``@rustbot`` label +F-cfg_version
r? ``@est31``
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) F-cfg_version `#![feature(cfg_version)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

6 participants