compiletest: accept optional whitespace in version range directives

[AayushMainali-Github]

extract_version_range previously split only on "- ", so directives like //@ ignore-llvm-version: 23-99 were treated as a single version (23) instead of a range. That silent misparse is what made #159331 necessary once LLVM trunk reached 24.

This change treats a dash with a digit on both sides (after trimming surrounding whitespace) as a range separator, so forms with or without spaces around the dash all work. Non-numeric suffixes such as 1.2.3-rc1 remain single versions.

Fixes #159338.

[rustbot]

Some changes occurred in src/tools/compiletest

cc @jieyouxu

compiletest directives have been modified. Please add or update docs for the
new or modified directive in src/doc/rustc-dev-guide/.

[rustbot]

Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @jieyouxu, @oli-obk, @wesleywiser, bootstrap
• @jieyouxu, @oli-obk, @wesleywiser, bootstrap expanded to 8 candidates
• Random selection from Mark-Simulacrum, clubby789, oli-obk, wesleywiser

[Mark-Simulacrum]

Presuming my grep isn't broken, there don't seem to be many version range directives... I'm wondering if we can restrict to known versions of some kind in the syntax? Or maybe switch to e.g. Rust range syntax (..) which is less likely to conflict?

$ rg 'version:' tests --no-filename | rg ':.*-'
//@ [aarch64-llvm-pre-23] ignore-llvm-version: 23 - 99
//@ ignore-gdb-version: 15.0 - 99.0
//@ ignore-gdb-version: 15.0 - 99.0
//@ ignore-gdb-version: 13.1 - 99.0
//@[llvm-current] ignore-llvm-version: 23 - 99
//@[loongarch32] ignore-llvm-version: 22 - 23
//@[loongarch64] ignore-llvm-version: 22 - 23
//@[loongarch32] ignore-llvm-version: 22 - 23
//@[loongarch64] ignore-llvm-version: 22 - 23

[jieyouxu]

//@ [aarch64-llvm-pre-23] ignore-llvm-version: 23 - 99

This should just be //@ max-llvm-major-version: 22 IMHO

//@ ignore-gdb-version: 15.0 - 99.0
//@ ignore-gdb-version: 15.0 - 99.0
//@ ignore-gdb-version: 13.1 - 99.0

I almost feel like these should just be //@ ignore-gdb (after #159451). Right now in CI we use the gdb that comes with Ubuntu 22.04, which happens to be gdb 12.1. It would be good to double-check why they require an older gdb rather than newer gdb. Also the end range of 99.0 is really just a funny way to say "ignore newer gdb versions than the version specified in the start range".

//@[llvm-current] ignore-llvm-version: 23 - 99
//@[loongarch32] ignore-llvm-version: 22 - 23
//@[loongarch64] ignore-llvm-version: 22 - 23
//@[loongarch32] ignore-llvm-version: 22 - 23
//@[loongarch64] ignore-llvm-version: 22 - 23

Ditto on //@ max-llvm-major-version, probably 22 and 21 respectively.

For the LLVM versioning, we can probably re-use the .. that //@ edition currently uses, cc

rust/src/tools/compiletest/src/directives.rs

Line 1288 in 234c31c

fn parse_edition_range(config: &Config, line: &DirectiveLine<'_>) -> Option<EditionRange> {

However I'm not actually convinced the ignore-llvm-version: 22 - 23 are good ignore ranges, do those test pass against... LLVM 24? LLVM trunk is only LLVM 23 right? I feel like those loongarch64 ones also want //@ max-llvm-major-version?

[AayushMainali-Github]

Thanks. Happy to change approach if preferred.

Options I’m seeing:

• Switch version ranges to .. (like //@ edition), and keep rejecting ambiguous forms
• Keep but require spaces and hard-error on 23-99 instead of accepting it
• replace X - 99 with max-llvm-major-version / simpler gdb ignores, and only touch the parser as needed

Which direction should this PR take? @Mark-Simulacrum @jieyouxu