Auto merge of #110319 - ferrocene:pa-more-ignore-reasons, r=ehuss

[compiletest] Add more test ignore reasons, `needs-` validation, and improved error messages

This PR makes more improvements to the way compiletest ignoring headers are handled, following up on #108905:

* Human-readable ignore reasons have been added for the remaining ignore causes (`needs-*` directives, `*llvm*` directives, and debugger version directives). All ignored tests should now have a human-readable reason.
* The code handling `needs-*` directives has been refactored, and now invalid `needs-*` directive emit errors like `ignore-*` and `only-*`.
* All errors are now displayed at startup (with line numbers) rather than just the first error of the first file.

This PR is best reviewed commit-by-commit.

r? `@ehuss`

Author: bors

src/tools/compiletest/src/header.rs

@@ -1,8 +1,43 @@
 use crate::common::{CompareMode, Config, Debugger};
+use crate::header::IgnoreDecision;
 use std::collections::HashSet;
 
 const EXTRA_ARCHS: &[&str] = &["spirv"];
 
+pub(super) fn handle_ignore(config: &Config, line: &str) -> IgnoreDecision {
+    let parsed = parse_cfg_name_directive(config, line, "ignore");
+    match parsed.outcome {
+        MatchOutcome::NoMatch => IgnoreDecision::Continue,
+        MatchOutcome::Match => IgnoreDecision::Ignore {
+            reason: match parsed.comment {
+                Some(comment) => format!("ignored {} ({comment})", parsed.pretty_reason.unwrap()),
+                None => format!("ignored {}", parsed.pretty_reason.unwrap()),
+            },
+        },
+        MatchOutcome::Invalid => IgnoreDecision::Error { message: format!("invalid line: {line}") },
+        MatchOutcome::External => IgnoreDecision::Continue,
+        MatchOutcome::NotADirective => IgnoreDecision::Continue,
+    }
+}
+
+pub(super) fn handle_only(config: &Config, line: &str) -> IgnoreDecision {
+    let parsed = parse_cfg_name_directive(config, line, "only");
+    match parsed.outcome {
+        MatchOutcome::Match => IgnoreDecision::Continue,
+        MatchOutcome::NoMatch => IgnoreDecision::Ignore {
+            reason: match parsed.comment {
+                Some(comment) => {
+                    format!("only executed {} ({comment})", parsed.pretty_reason.unwrap())
+                }
+                None => format!("only executed {}", parsed.pretty_reason.unwrap()),
+            },
+        },
+        MatchOutcome::Invalid => IgnoreDecision::Error { message: format!("invalid line: {line}") },
+        MatchOutcome::External => IgnoreDecision::Continue,
+        MatchOutcome::NotADirective => IgnoreDecision::Continue,
+    }
+}
+
 /// Parses a name-value directive which contains config-specific information, e.g., `ignore-x86`
 /// or `normalize-stderr-32bit`.
 pub(super) fn parse_cfg_name_directive<'a>(
@@ -11,10 +46,10 @@ pub(super) fn parse_cfg_name_directive<'a>(
     prefix: &str,
 ) -> ParsedNameDirective<'a> {
     if !line.as_bytes().starts_with(prefix.as_bytes()) {
-        return ParsedNameDirective::invalid();
+        return ParsedNameDirective::not_a_directive();
     }
     if line.as_bytes().get(prefix.len()) != Some(&b'-') {
-        return ParsedNameDirective::invalid();
+        return ParsedNameDirective::not_a_directive();
     }
     let line = &line[prefix.len() + 1..];
 
@@ -24,7 +59,7 @@ pub(super) fn parse_cfg_name_directive<'a>(
     // Some of the matchers might be "" depending on what the target information is. To avoid
     // problems we outright reject empty directives.
     if name == "" {
-        return ParsedNameDirective::invalid();
+        return ParsedNameDirective::not_a_directive();
     }
 
     let mut outcome = MatchOutcome::Invalid;
@@ -218,8 +253,13 @@ pub(super) struct ParsedNameDirective<'a> {
 }
 
 impl ParsedNameDirective<'_> {
-    fn invalid() -> Self {
-        Self { name: None, pretty_reason: None, comment: None, outcome: MatchOutcome::NoMatch }
+    fn not_a_directive() -> Self {
+        Self {
+            name: None,
+            pretty_reason: None,
+            comment: None,
+            outcome: MatchOutcome::NotADirective,
+        }
     }
 }
 
@@ -233,6 +273,8 @@ pub(super) enum MatchOutcome {
     Invalid,
     /// The directive is handled by other parts of our tooling.
     External,
+    /// The line is not actually a directive.
+    NotADirective,
 }
 
 trait CustomContains {

src/tools/compiletest/src/header/cfg.rs

@@ -0,0 +1,236 @@
+use crate::common::{Config, Debugger};
+use crate::header::IgnoreDecision;
+use crate::util;
+
+pub(super) fn handle_needs(
+    cache: &CachedNeedsConditions,
+    config: &Config,
+    ln: &str,
+) -> IgnoreDecision {
+    // Note thet we intentionally still put the needs- prefix here to make the file show up when
+    // grepping for a directive name, even though we could technically strip that.
+    let needs = &[
+        Need {
+            name: "needs-asm-support",
+            condition: config.has_asm_support(),
+            ignore_reason: "ignored on targets without inline assembly support",
+        },
+        Need {
+            name: "needs-sanitizer-support",
+            condition: cache.sanitizer_support,
+            ignore_reason: "ignored on targets without sanitizers support",
+        },
+        Need {
+            name: "needs-sanitizer-address",
+            condition: cache.sanitizer_address,
+            ignore_reason: "ignored on targets without address sanitizer",
+        },
+        Need {
+            name: "needs-sanitizer-cfi",
+            condition: cache.sanitizer_cfi,
+            ignore_reason: "ignored on targets without CFI sanitizer",
+        },
+        Need {
+            name: "needs-sanitizer-kcfi",
+            condition: cache.sanitizer_kcfi,
+            ignore_reason: "ignored on targets without kernel CFI sanitizer",
+        },
+        Need {
+            name: "needs-sanitizer-kasan",
+            condition: cache.sanitizer_kasan,
+            ignore_reason: "ignored on targets without kernel address sanitizer",
+        },
+        Need {
+            name: "needs-sanitizer-leak",
+            condition: cache.sanitizer_leak,
+            ignore_reason: "ignored on targets without leak sanitizer",
+        },
+        Need {
+            name: "needs-sanitizer-memory",
+            condition: cache.sanitizer_memory,
+            ignore_reason: "ignored on targets without memory sanitizer",
+        },
+        Need {
+            name: "needs-sanitizer-thread",
+            condition: cache.sanitizer_thread,
+            ignore_reason: "ignored on targets without thread sanitizer",
+        },
+        Need {
+            name: "needs-sanitizer-hwaddress",
+            condition: cache.sanitizer_hwaddress,
+            ignore_reason: "ignored on targets without hardware-assisted address sanitizer",
+        },
+        Need {
+            name: "needs-sanitizer-memtag",
+            condition: cache.sanitizer_memtag,
+            ignore_reason: "ignored on targets without memory tagging sanitizer",
+        },
+        Need {
+            name: "needs-sanitizer-shadow-call-stack",
+            condition: cache.sanitizer_shadow_call_stack,
+            ignore_reason: "ignored on targets without shadow call stacks",
+        },
+        Need {
+            name: "needs-run-enabled",
+            condition: config.run_enabled(),
+            ignore_reason: "ignored when running the resulting test binaries is disabled",
+        },
+        Need {
+            name: "needs-unwind",
+            condition: config.can_unwind(),
+            ignore_reason: "ignored on targets without unwinding support",
+        },
+        Need {
+            name: "needs-profiler-support",
+            condition: std::env::var_os("RUSTC_PROFILER_SUPPORT").is_some(),
+            ignore_reason: "ignored when profiler support is disabled",
+        },
+        Need {
+            name: "needs-matching-clang",
+            condition: config.run_clang_based_tests_with.is_some(),
+            ignore_reason: "ignored when the used clang does not match the built LLVM",
+        },
+        Need {
+            name: "needs-xray",
+            condition: cache.xray,
+            ignore_reason: "ignored on targets without xray tracing",
+        },
+        Need {
+            name: "needs-rust-lld",
+            condition: cache.rust_lld,
+            ignore_reason: "ignored on targets without Rust's LLD",
+        },
+        Need {
+            name: "needs-rust-lldb",
+            condition: config.debugger != Some(Debugger::Lldb) || config.lldb_native_rust,
+            ignore_reason: "ignored on targets without Rust's LLDB",
+        },
+        Need {
+            name: "needs-i686-dlltool",
+            condition: cache.i686_dlltool,
+            ignore_reason: "ignored when dlltool for i686 is not present",
+        },
+        Need {
+            name: "needs-x86_64-dlltool",
+            condition: cache.x86_64_dlltool,
+            ignore_reason: "ignored when dlltool for x86_64 is not present",
+        },
+    ];
+
+    let (name, comment) = match ln.split_once([':', ' ']) {
+        Some((name, comment)) => (name, Some(comment)),
+        None => (ln, None),
+    };
+
+    if !name.starts_with("needs-") {
+        return IgnoreDecision::Continue;
+    }
+
+    // Handled elsewhere.
+    if name == "needs-llvm-components" {
+        return IgnoreDecision::Continue;
+    }
+
+    let mut found_valid = false;
+    for need in needs {
+        if need.name == name {
+            if need.condition {
+                found_valid = true;
+                break;
+            } else {
+                return IgnoreDecision::Ignore {
+                    reason: if let Some(comment) = comment {
+                        format!("{} ({comment})", need.ignore_reason)
+                    } else {
+                        need.ignore_reason.into()
+                    },
+                };
+            }
+        }
+    }
+
+    if found_valid {
+        IgnoreDecision::Continue
+    } else {
+        IgnoreDecision::Error { message: format!("invalid needs directive: {name}") }
+    }
+}
+
+struct Need {
+    name: &'static str,
+    condition: bool,
+    ignore_reason: &'static str,
+}
+
+pub(super) struct CachedNeedsConditions {
+    sanitizer_support: bool,
+    sanitizer_address: bool,
+    sanitizer_cfi: bool,
+    sanitizer_kcfi: bool,
+    sanitizer_kasan: bool,
+    sanitizer_leak: bool,
+    sanitizer_memory: bool,
+    sanitizer_thread: bool,
+    sanitizer_hwaddress: bool,
+    sanitizer_memtag: bool,
+    sanitizer_shadow_call_stack: bool,
+    xray: bool,
+    rust_lld: bool,
+    i686_dlltool: bool,
+    x86_64_dlltool: bool,
+}
+
+impl CachedNeedsConditions {
+    pub(super) fn load(config: &Config) -> Self {
+        let path = std::env::var_os("PATH").expect("missing PATH environment variable");
+        let path = std::env::split_paths(&path).collect::<Vec<_>>();
+
+        let target = &&*config.target;
+        Self {
+            sanitizer_support: std::env::var_os("RUSTC_SANITIZER_SUPPORT").is_some(),
+            sanitizer_address: util::ASAN_SUPPORTED_TARGETS.contains(target),
+            sanitizer_cfi: util::CFI_SUPPORTED_TARGETS.contains(target),
+            sanitizer_kcfi: util::KCFI_SUPPORTED_TARGETS.contains(target),
+            sanitizer_kasan: util::KASAN_SUPPORTED_TARGETS.contains(target),
+            sanitizer_leak: util::LSAN_SUPPORTED_TARGETS.contains(target),
+            sanitizer_memory: util::MSAN_SUPPORTED_TARGETS.contains(target),
+            sanitizer_thread: util::TSAN_SUPPORTED_TARGETS.contains(target),
+            sanitizer_hwaddress: util::HWASAN_SUPPORTED_TARGETS.contains(target),
+            sanitizer_memtag: util::MEMTAG_SUPPORTED_TARGETS.contains(target),
+            sanitizer_shadow_call_stack: util::SHADOWCALLSTACK_SUPPORTED_TARGETS.contains(target),
+            xray: util::XRAY_SUPPORTED_TARGETS.contains(target),
+
+            // For tests using the `needs-rust-lld` directive (e.g. for `-Zgcc-ld=lld`), we need to find
+            // whether `rust-lld` is present in the compiler under test.
+            //
+            // The --compile-lib-path is the path to host shared libraries, but depends on the OS. For
+            // example:
+            // - on linux, it can be <sysroot>/lib
+            // - on windows, it can be <sysroot>/bin
+            //
+            // However, `rust-lld` is only located under the lib path, so we look for it there.
+            rust_lld: config
+                .compile_lib_path
+                .parent()
+                .expect("couldn't traverse to the parent of the specified --compile-lib-path")
+                .join("lib")
+                .join("rustlib")
+                .join(target)
+                .join("bin")
+                .join(if config.host.contains("windows") { "rust-lld.exe" } else { "rust-lld" })
+                .exists(),
+
+            // On Windows, dlltool.exe is used for all architectures.
+            #[cfg(windows)]
+            i686_dlltool: path.iter().any(|dir| dir.join("dlltool.exe").is_file()),
+            #[cfg(windows)]
+            x86_64_dlltool: path.iter().any(|dir| dir.join("dlltool.exe").is_file()),
+
+            // For non-Windows, there are architecture specific dlltool binaries.
+            #[cfg(not(windows))]
+            i686_dlltool: path.iter().any(|dir| dir.join("i686-w64-mingw32-dlltool").is_file()),
+            #[cfg(not(windows))]
+            x86_64_dlltool: path.iter().any(|dir| dir.join("x86_64-w64-mingw32-dlltool").is_file()),
+        }
+    }
+}

src/tools/compiletest/src/header/needs.rs

@@ -1,7 +1,25 @@
+use std::io::Read;
 use std::path::Path;
 
 use crate::common::{Config, Debugger};
-use crate::header::{make_test_description, parse_normalization_string, EarlyProps};
+use crate::header::{parse_normalization_string, EarlyProps, HeadersCache};
+
+fn make_test_description<R: Read>(
+    config: &Config,
+    name: test::TestName,
+    path: &Path,
+    src: R,
+    cfg: Option<&str>,
+) -> test::TestDesc {
+    let cache = HeadersCache::load(config);
+    let mut poisoned = false;
+    let test =
+        crate::header::make_test_description(config, &cache, name, path, src, cfg, &mut poisoned);
+    if poisoned {
+        panic!("poisoned!");
+    }
+    test
+}
 
 #[test]
 fn test_parse_normalization_string() {