Skip to content

[WIP] Normalize rust version string centrally in compiletest #142940

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

Closed
wants to merge 5 commits into from

Conversation

jieyouxu
Copy link
Member

Caution

Stacked on top of #142930, that needs to merge first then this PR needs to be rebased.

Instead of maintaining n copies of

//@ normalize-stderr: "you are using [0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9]+)?( \([^)]*\))?" -> "you are using $$RUSTC_VERSION"

Normalize this consistently and centrally in compiletest instead.

cc @cuviper
r? @Kobzol

cuviper and others added 2 commits June 23, 2025 13:30
Several UI tests have a `normalize-stderr` for "you are using x.y.z"
rustc versions, and that regex is flexible enough for suffixes like
"-nightly" and "-dev", but not for "-beta.N". We can just add '.' to
that trailing pattern to include this.
@rustbot rustbot added A-compiletest Area: The compiletest test runner A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 24, 2025
@jieyouxu
Copy link
Member Author

I believe this normalization can technically fail if you try to run these ui tests against a stage 0 beta compiler (which has a different version string than stage 1+ compiler that uses the "current" version string). However I don't think ui test suite supports being run against stage 0 compiler, surely quite a few ui tests will fail due to compiler differences themselves.

@rust-log-analyzer

This comment has been minimized.

@workingjubilee
Copy link
Member

There is a test that tests cfg(version(N)) which will be at risk from this.

@jieyouxu jieyouxu force-pushed the normalize-rustc-version branch from 002c00b to 477627d Compare June 24, 2025 01:55
@jieyouxu
Copy link
Member Author

There is a test that tests cfg(version(N)) which will be at risk from this.

Hm, good point, let me double-check.

@jieyouxu
Copy link
Member Author

jieyouxu commented Jun 24, 2025

You're right. We can't naively do the normalization like in this PR. Some options:

  1. Just leave the normalization regexes as-is. Those are... convoluted, but localized. Though future test writers need to remember to have that normalization rule.
  2. Normalize for ui mode by default, but opt-out with another directive. Convoluted, and easy to forget.
  3. Don't show the "current" rustc version in the diagnostics (i.e. trim out the current rustc version note in Note the version and PR of removed features when using it #141642).

Honestly, I'm leaning towards option (3).

@workingjubilee
Copy link
Member

option 3 seems like the thing to do, yes.

@jieyouxu
Copy link
Member Author

I'm going to close this PR, and instead go with option (3) in another PR. IMO it's not a huge loss, because you can just rustc --version (or cargo --version).

@jieyouxu jieyouxu closed this Jun 24, 2025
@jieyouxu jieyouxu deleted the normalize-rustc-version branch June 24, 2025 02:05
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 24, 2025
…ler-errors

Don't include current rustc version string in feature removed help

The version string is difficult to properly normalize out, and removing it isn't a huge deal (the user can query version info easily through `rustc --version` or `cargo --version`).

The normalization options were all non-ideal (see rust-lang#142940 (comment)):

- Per-test version string normalization is nasty to maintain, and we  need to maintain `n` copies of it. See rust-lang#142930 where the regex wasn't  robust against different release channels.
- Centralized compiletest normalization (with a directive opt-out) is  also not ideal, because `cfg(version(..))` tests can't have those accidentally normalized out (and you'd have to remember to opt-out).

r? `@workingjubilee` (discussed in rust-lang#142940)
rust-timer added a commit that referenced this pull request Jun 25, 2025
Rollup merge of #142943 - jieyouxu:no-rustc-version, r=compiler-errors

Don't include current rustc version string in feature removed help

The version string is difficult to properly normalize out, and removing it isn't a huge deal (the user can query version info easily through `rustc --version` or `cargo --version`).

The normalization options were all non-ideal (see #142940 (comment)):

- Per-test version string normalization is nasty to maintain, and we  need to maintain `n` copies of it. See #142930 where the regex wasn't  robust against different release channels.
- Centralized compiletest normalization (with a directive opt-out) is  also not ideal, because `cfg(version(..))` tests can't have those accidentally normalized out (and you'd have to remember to opt-out).

r? `@workingjubilee` (discussed in #142940)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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