From ff0c0967c15df8c714195905d51719db80fe5a53 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 20 Oct 2025 15:31:18 +1100 Subject: [PATCH 1/5] Add support for `//@ ignore-i586` There are a few tests that were trying to skip i586 targets via the `TARGET` environment variable instead, so better to just add support for the directive. --- src/tools/compiletest/src/common.rs | 11 +++++++---- src/tools/compiletest/src/directives/cfg.rs | 8 ++++++++ .../compiletest/src/directives/directive_names.rs | 1 + src/tools/compiletest/src/directives/tests.rs | 2 ++ tests/assembly-llvm/x86-return-float.rs | 5 +---- .../ui/abi/homogenous-floats-target-feature-mixup.rs | 11 +---------- tests/ui/target-feature/target-feature-detection.rs | 12 +----------- 7 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 58330ed20dc88..0a2f4340bbea1 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -701,10 +701,13 @@ impl Config { } pub fn matches_arch(&self, arch: &str) -> bool { - self.target_cfg().arch == arch || - // Matching all the thumb variants as one can be convenient. - // (thumbv6m, thumbv7em, thumbv7m, etc.) - (arch == "thumb" && self.target.starts_with("thumb")) + self.target_cfg().arch == arch + || { + // Matching all the thumb variants as one can be convenient. + // (thumbv6m, thumbv7em, thumbv7m, etc.) + arch == "thumb" && self.target.starts_with("thumb") + } + || (arch == "i586" && self.target.starts_with("i586-")) } pub fn matches_os(&self, os: &str) -> bool { diff --git a/src/tools/compiletest/src/directives/cfg.rs b/src/tools/compiletest/src/directives/cfg.rs index 3855ba7ac5f42..412b8ebaa068e 100644 --- a/src/tools/compiletest/src/directives/cfg.rs +++ b/src/tools/compiletest/src/directives/cfg.rs @@ -166,6 +166,14 @@ fn parse_cfg_name_directive<'a>( message: "when the architecture is part of the Thumb family" } + // The "arch" of `i586-` targets is "x86", so for more specific matching + // we have to resort to a string-prefix check. + condition! { + name: "i586", + condition: config.matches_arch("i586"), + message: "when the subarchitecture is i586", + } + condition! { name: "apple", condition: config.target.contains("apple"), diff --git a/src/tools/compiletest/src/directives/directive_names.rs b/src/tools/compiletest/src/directives/directive_names.rs index 1ab73fd87e1d0..3a46dbc704e87 100644 --- a/src/tools/compiletest/src/directives/directive_names.rs +++ b/src/tools/compiletest/src/directives/directive_names.rs @@ -70,6 +70,7 @@ pub(crate) const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "ignore-gnu", "ignore-haiku", "ignore-horizon", + "ignore-i586", "ignore-i686-pc-windows-gnu", "ignore-i686-pc-windows-msvc", "ignore-illumos", diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index ddc14b96a7de3..98249e69601be 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -605,6 +605,8 @@ fn ignore_arch() { ("i686-unknown-linux-gnu", "x86"), ("nvptx64-nvidia-cuda", "nvptx64"), ("thumbv7m-none-eabi", "thumb"), + ("i586-unknown-linux-gnu", "x86"), + ("i586-unknown-linux-gnu", "i586"), ]; for (target, arch) in archs { let config: Config = cfg().target(target).build(); diff --git a/tests/assembly-llvm/x86-return-float.rs b/tests/assembly-llvm/x86-return-float.rs index 4db93f68485a3..1b283c2759b3a 100644 --- a/tests/assembly-llvm/x86-return-float.rs +++ b/tests/assembly-llvm/x86-return-float.rs @@ -1,9 +1,6 @@ //@ assembly-output: emit-asm // FIXME(#114479): LLVM miscompiles loading and storing `f32` and `f64` when SSE is disabled. -// There's no compiletest directive to ignore a test on i586 only, so just always explicitly enable -// SSE2. -// Use the same target CPU as `i686` so that LLVM orders the instructions in the same order. -//@ compile-flags: -Ctarget-feature=+sse2 -Ctarget-cpu=pentium4 +// (As of #136758, this test cross-compiles to selected i686 targets only, which have SSE.) // Force frame pointers to make ASM more consistent between targets //@ compile-flags: -C force-frame-pointers // At opt-level=3, LLVM can merge two movss into one movsd, and we aren't testing for that. diff --git a/tests/ui/abi/homogenous-floats-target-feature-mixup.rs b/tests/ui/abi/homogenous-floats-target-feature-mixup.rs index f472b1aa9aff2..21c6c276bb1c7 100644 --- a/tests/ui/abi/homogenous-floats-target-feature-mixup.rs +++ b/tests/ui/abi/homogenous-floats-target-feature-mixup.rs @@ -7,6 +7,7 @@ //@ run-pass //@ needs-subprocess //@ ignore-backends: gcc +//@ ignore-i586 (no SSE2) #![allow(overflowing_literals)] #![allow(unused_variables)] @@ -19,16 +20,6 @@ fn main() { return test::main(&level) } - match std::env::var("TARGET") { - Ok(s) => { - // Skip this tests on i586-unknown-linux-gnu where sse2 is disabled - if s.contains("i586") { - return - } - } - Err(_) => return, - } - let me = env::current_exe().unwrap(); for level in ["sse", "avx", "avx512"].iter() { let status = Command::new(&me).arg(level).status().unwrap(); diff --git a/tests/ui/target-feature/target-feature-detection.rs b/tests/ui/target-feature/target-feature-detection.rs index 3404bfbe782ac..41fc246913125 100644 --- a/tests/ui/target-feature/target-feature-detection.rs +++ b/tests/ui/target-feature/target-feature-detection.rs @@ -2,22 +2,12 @@ //! specifically `sse2` on x86/x86_64 platforms, and correctly reports absent features. //@ run-pass +//@ ignore-i586 (no SSE2) #![allow(stable_features)] #![feature(cfg_target_feature)] -use std::env; - fn main() { - match env::var("TARGET") { - Ok(s) => { - // Skip this tests on i586-unknown-linux-gnu where sse2 is disabled - if s.contains("i586") { - return; - } - } - Err(_) => return, - } if cfg!(any(target_arch = "x86", target_arch = "x86_64")) { assert!( cfg!(target_feature = "sse2"), From a58115426e97349b2583211c410e916d8e932034 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 20 Oct 2025 13:05:25 +1100 Subject: [PATCH 2/5] Don't set `TARGET` for non run-make tests If something like this is needed again in the future, please set environment variables for the build and run subprocesses, instead of setting them on the compiletest process. --- src/tools/compiletest/src/lib.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 798008ab432ed..11a462c3d44a6 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -525,11 +525,6 @@ fn run_tests(config: Arc) { // SAFETY: at this point we're still single-threaded. unsafe { env::set_var("__COMPAT_LAYER", "RunAsInvoker") }; - // Let tests know which target they're running as. - // - // SAFETY: at this point we're still single-threaded. - unsafe { env::set_var("TARGET", &config.target) }; - let mut configs = Vec::new(); if let TestMode::DebugInfo = config.mode { // Debugging emscripten code doesn't make sense today From b39fb327cbd2fb6edf68ea8fb0ba1494ac614f3d Mon Sep 17 00:00:00 2001 From: Thalia Archibald Date: Mon, 20 Oct 2025 21:45:14 -0600 Subject: [PATCH 3/5] motor: Add new set_times stubs --- library/std/src/sys/fs/motor.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/std/src/sys/fs/motor.rs b/library/std/src/sys/fs/motor.rs index 656b6e81b9518..f723a23bd5dff 100644 --- a/library/std/src/sys/fs/motor.rs +++ b/library/std/src/sys/fs/motor.rs @@ -324,6 +324,14 @@ pub fn set_perm(path: &Path, perm: FilePermissions) -> io::Result<()> { moto_rt::fs::set_perm(path, perm.rt_perm).map_err(map_motor_error) } +pub fn set_times(_p: &Path, _times: FileTimes) -> io::Result<()> { + unsupported() +} + +pub fn set_times_nofollow(_p: &Path, _times: FileTimes) -> io::Result<()> { + unsupported() +} + pub fn readlink(_p: &Path) -> io::Result { unsupported() } From 891b8475bd31da09f78f08123347514fbd0491ff Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 21 Oct 2025 16:52:44 +1100 Subject: [PATCH 4/5] Remove a confused use of `revision` from `update_pass_mode` --- src/tools/compiletest/src/directives.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 3bb5933553f12..343ae854b7d5e 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -504,7 +504,7 @@ impl TestProps { &mut self.check_test_line_numbers_match, ); - self.update_pass_mode(ln, test_revision, config); + self.update_pass_mode(ln, config); self.update_fail_mode(ln, config); config.set_name_directive(ln, IGNORE_PASS, &mut self.ignore_pass); @@ -714,18 +714,15 @@ impl TestProps { } } - fn update_pass_mode( - &mut self, - ln: &DirectiveLine<'_>, - revision: Option<&str>, - config: &Config, - ) { + fn update_pass_mode(&mut self, ln: &DirectiveLine<'_>, config: &Config) { let check_no_run = |s| match (config.mode, s) { (TestMode::Ui, _) => (), (TestMode::Crashes, _) => (), (TestMode::Codegen, "build-pass") => (), (TestMode::Incremental, _) => { - if revision.is_some() && !self.revisions.iter().all(|r| r.starts_with("cfail")) { + // FIXME(Zalathar): This only detects forbidden directives that are + // declared _after_ the incompatible `//@ revisions:` directive(s). + if self.revisions.iter().any(|r| !r.starts_with("cfail")) { panic!("`{s}` directive is only supported in `cfail` incremental tests") } } From 860a84c04afb53e2c32e566c7726a377e1f2e9ab Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 21 Oct 2025 17:10:04 +1100 Subject: [PATCH 5/5] Remove bad-directive detection that has been subsumed by the allowlist Now that all directive names are checked against a list of known-good names, these ad-hoc checks can never fire. --- src/tools/compiletest/src/directives.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 343ae854b7d5e..51eac58c971eb 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -256,8 +256,6 @@ mod directives { pub const NO_AUTO_CHECK_CFG: &'static str = "no-auto-check-cfg"; pub const ADD_CORE_STUBS: &'static str = "add-core-stubs"; pub const CORE_STUBS_COMPILE_FLAGS: &'static str = "core-stubs-compile-flags"; - // This isn't a real directive, just one that is probably mistyped often - pub const INCORRECT_COMPILER_FLAGS: &'static str = "compiler-flags"; pub const DISABLE_GDB_PRETTY_PRINTERS: &'static str = "disable-gdb-pretty-printers"; pub const COMPARE_OUTPUT_BY_LINES: &'static str = "compare-output-by-lines"; } @@ -418,9 +416,6 @@ impl TestProps { } self.compile_flags.extend(flags); } - if config.parse_name_value_directive(ln, INCORRECT_COMPILER_FLAGS).is_some() { - panic!("`compiler-flags` directive should be spelled `compile-flags`"); - } if let Some(range) = parse_edition_range(config, ln) { self.edition = Some(range.edition_to_test(config.edition)); @@ -686,9 +681,6 @@ impl TestProps { panic!("`{}-fail` directive is only supported in UI tests", mode); } }; - if config.mode == TestMode::Ui && config.parse_name_directive(ln, "compile-fail") { - panic!("`compile-fail` directive is useless in UI tests"); - } let fail_mode = if config.parse_name_directive(ln, "check-fail") { check_ui("check"); Some(FailMode::Check) @@ -809,6 +801,11 @@ fn check_directive<'a>( .map(|remark| remark.trim_start().split(' ').next().unwrap()) .filter(|token| KNOWN_DIRECTIVE_NAMES.contains(token)); + // FIXME(Zalathar): Consider emitting specialized error/help messages for + // bogus directive names that are similar to real ones, e.g.: + // - *`compiler-flags` => `compile-flags` + // - *`compile-fail` => `check-fail` or `build-fail` + CheckDirectiveResult { is_known_directive, trailing_directive } }