[COMPILETEST-UNTANGLE 5/N] Test mode adjustments and other assorted cleanups

src/tools/compiletest/src/common.rs

@@ -1,63 +1,20 @@
 use std::collections::{BTreeSet, HashMap, HashSet};
+use std::iter;
 use std::process::Command;
-use std::str::FromStr;
 use std::sync::OnceLock;
-use std::{fmt, iter};
 
 use build_helper::git::GitConfig;
 use camino::{Utf8Path, Utf8PathBuf};
 use semver::Version;
 use serde::de::{Deserialize, Deserializer, Error as _};
 
-pub use self::Mode::*;
 use crate::executor::{ColorConfig, OutputFormat};
 use crate::fatal;
-use crate::util::{Utf8PathBufExt, add_dylib_path};
-
-macro_rules! string_enum {
-    ($(#[$meta:meta])* $vis:vis enum $name:ident { $($variant:ident => $repr:expr,)* }) => {
-        $(#[$meta])*
-        $vis enum $name {
-            $($variant,)*
-        }
-
-        impl $name {
-            $vis const VARIANTS: &'static [Self] = &[$(Self::$variant,)*];
-            $vis const STR_VARIANTS: &'static [&'static str] = &[$(Self::$variant.to_str(),)*];
-
-            $vis const fn to_str(&self) -> &'static str {
-                match self {
-                    $(Self::$variant => $repr,)*
-                }
-            }
-        }
-
-        impl fmt::Display for $name {
-            fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-                fmt::Display::fmt(self.to_str(), f)
-            }
-        }
-
-        impl FromStr for $name {
-            type Err = String;
-
-            fn from_str(s: &str) -> Result<Self, Self::Err> {
-                match s {
-                    $($repr => Ok(Self::$variant),)*
-                    _ => Err(format!(concat!("unknown `", stringify!($name), "` variant: `{}`"), s)),
-                }
-            }
-        }
-    }
-}
-
-// Make the macro visible outside of this module, for tests.
-#[cfg(test)]
-pub(crate) use string_enum;
+use crate::util::{Utf8PathBufExt, add_dylib_path, string_enum};
 
 string_enum! {
     #[derive(Clone, Copy, PartialEq, Debug)]
-    pub enum Mode {
+    pub enum TestMode {
         Pretty => "pretty",
         DebugInfo => "debuginfo",
         Codegen => "codegen",
@@ -76,18 +33,12 @@ string_enum! {
     }
 }
 
-impl Default for Mode {
-    fn default() -> Self {
-        Mode::Ui
-    }
-}
-
-impl Mode {
+impl TestMode {
     pub fn aux_dir_disambiguator(self) -> &'static str {
         // Pretty-printing tests could run concurrently, and if they do,
         // they need to keep their output segregated.
         match self {
-            Pretty => ".pretty",
+            TestMode::Pretty => ".pretty",
             _ => "",
         }
     }
@@ -96,7 +47,7 @@ impl Mode {
         // Coverage tests use the same test files for multiple test modes,
         // so each mode should have a separate output directory.
         match self {
-            CoverageMap | CoverageRun => self.to_str(),
+            TestMode::CoverageMap | TestMode::CoverageRun => self.to_str(),
             _ => "",
         }
     }
@@ -193,9 +144,9 @@ pub enum Sanitizer {
 ///
 /// FIXME: audit these options to make sure we are not hashing less than necessary for build stamp
 /// (for changed test detection).
-#[derive(Debug, Default, Clone)]
+#[derive(Debug, Clone)]
 pub struct Config {
-    /// Some test [`Mode`]s support [snapshot testing], where a *reference snapshot* of outputs (of
+    /// Some [`TestMode`]s support [snapshot testing], where a *reference snapshot* of outputs (of
     /// `stdout`, `stderr`, or other form of artifacts) can be compared to the *actual output*.
     ///
     /// This option can be set to `true` to update the *reference snapshots* in-place, otherwise
@@ -317,20 +268,20 @@ pub struct Config {
     /// FIXME: reconsider this string; this is hashed for test build stamp.
     pub stage_id: String,
 
-    /// The test [`Mode`]. E.g. [`Mode::Ui`]. Each test mode can correspond to one or more test
+    /// The [`TestMode`]. E.g. [`TestMode::Ui`]. Each test mode can correspond to one or more test
     /// suites.
     ///
     /// FIXME: stop using stringly-typed test suites!
-    pub mode: Mode,
+    pub mode: TestMode,
 
     /// The test suite.
     ///
-    /// Example: `tests/ui/` is the "UI" test *suite*, which happens to also be of the [`Mode::Ui`]
-    /// test *mode*.
+    /// Example: `tests/ui/` is the "UI" test *suite*, which happens to also be of the
+    /// [`TestMode::Ui`] test *mode*.
     ///
     /// Note that the same test directory (e.g. `tests/coverage/`) may correspond to multiple test
-    /// modes, e.g. `tests/coverage/` can be run under both [`Mode::CoverageRun`] and
-    /// [`Mode::CoverageMap`].
+    /// modes, e.g. `tests/coverage/` can be run under both [`TestMode::CoverageRun`] and
+    /// [`TestMode::CoverageMap`].
     ///
     /// FIXME: stop using stringly-typed test suites!
     pub suite: String,
@@ -586,8 +537,8 @@ pub struct Config {
     // Configuration for various run-make tests frobbing things like C compilers or querying about
     // various LLVM component information.
     //
-    // FIXME: this really should be better packaged together.
-    // FIXME: these need better docs, e.g. for *host*, or for *target*?
+    // FIXME: this really should be better packaged together. FIXME: these need better docs, e.g.
+    // for *host*, or for *target*?
     pub cc: String,
     pub cxx: String,
     pub cflags: String,

src/tools/compiletest/src/debuggers.rs

@@ -51,17 +51,6 @@ pub(crate) fn configure_gdb(config: &Config) -> Option<Arc<Config>> {
 pub(crate) fn configure_lldb(config: &Config) -> Option<Arc<Config>> {
     config.lldb_python_dir.as_ref()?;
 
-    // FIXME: this is super old
-    if let Some(350) = config.lldb_version {
-        println!(
-            "WARNING: The used version of LLDB (350) has a \
-             known issue that breaks debuginfo tests. See \
-             issue #32520 for more information. Skipping all \
-             LLDB-based tests!",
-        );
-        return None;
-    }
-
     Some(Arc::new(Config { debugger: Some(Debugger::Lldb), ..config.clone() }))
 }
 

src/tools/compiletest/src/directives.rs

@@ -9,7 +9,7 @@ use camino::{Utf8Path, Utf8PathBuf};
 use semver::Version;
 use tracing::*;
 
-use crate::common::{Config, Debugger, FailMode, Mode, PassMode};
+use crate::common::{Config, Debugger, FailMode, PassMode, TestMode};
 use crate::debuggers::{extract_cdb_version, extract_gdb_version};
 use crate::directives::auxiliary::{AuxProps, parse_and_update_aux};
 use crate::directives::needs::CachedNeedsConditions;
@@ -328,7 +328,7 @@ impl TestProps {
         props.exec_env.push(("RUSTC".to_string(), config.rustc_path.to_string()));
 
         match (props.pass_mode, props.fail_mode) {
-            (None, None) if config.mode == Mode::Ui => props.fail_mode = Some(FailMode::Check),
+            (None, None) if config.mode == TestMode::Ui => props.fail_mode = Some(FailMode::Check),
             (Some(_), Some(_)) => panic!("cannot use a *-fail and *-pass mode together"),
             _ => {}
         }
@@ -609,11 +609,11 @@ impl TestProps {
             self.failure_status = Some(101);
         }
 
-        if config.mode == Mode::Incremental {
+        if config.mode == TestMode::Incremental {
             self.incremental = true;
         }
 
-        if config.mode == Mode::Crashes {
+        if config.mode == TestMode::Crashes {
             // we don't want to pollute anything with backtrace-files
             // also turn off backtraces in order to save some execution
             // time on the tests; we only need to know IF it crashes
@@ -641,11 +641,11 @@ impl TestProps {
     fn update_fail_mode(&mut self, ln: &str, config: &Config) {
         let check_ui = |mode: &str| {
             // Mode::Crashes may need build-fail in order to trigger llvm errors or stack overflows
-            if config.mode != Mode::Ui && config.mode != Mode::Crashes {
+            if config.mode != TestMode::Ui && config.mode != TestMode::Crashes {
                 panic!("`{}-fail` directive is only supported in UI tests", mode);
             }
         };
-        if config.mode == Mode::Ui && config.parse_name_directive(ln, "compile-fail") {
+        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") {
@@ -669,10 +669,10 @@ impl TestProps {
 
     fn update_pass_mode(&mut self, ln: &str, revision: Option<&str>, config: &Config) {
         let check_no_run = |s| match (config.mode, s) {
-            (Mode::Ui, _) => (),
-            (Mode::Crashes, _) => (),
-            (Mode::Codegen, "build-pass") => (),
-            (Mode::Incremental, _) => {
+            (TestMode::Ui, _) => (),
+            (TestMode::Crashes, _) => (),
+            (TestMode::Codegen, "build-pass") => (),
+            (TestMode::Incremental, _) => {
                 if revision.is_some() && !self.revisions.iter().all(|r| r.starts_with("cfail")) {
                     panic!("`{s}` directive is only supported in `cfail` incremental tests")
                 }
@@ -715,7 +715,7 @@ impl TestProps {
     pub fn update_add_core_stubs(&mut self, ln: &str, config: &Config) {
         let add_core_stubs = config.parse_name_directive(ln, directives::ADD_CORE_STUBS);
         if add_core_stubs {
-            if !matches!(config.mode, Mode::Ui | Mode::Codegen | Mode::Assembly) {
+            if !matches!(config.mode, TestMode::Ui | TestMode::Codegen | TestMode::Assembly) {
                 panic!(
                     "`add-core-stubs` is currently only supported for ui, codegen and assembly test modes"
                 );
@@ -833,7 +833,7 @@ pub(crate) struct CheckDirectiveResult<'ln> {
 
 pub(crate) fn check_directive<'a>(
     directive_ln: &'a str,
-    mode: Mode,
+    mode: TestMode,
     original_line: &str,
 ) -> CheckDirectiveResult<'a> {
     let (directive_name, post) = directive_ln.split_once([':', ' ']).unwrap_or((directive_ln, ""));
@@ -842,11 +842,11 @@ pub(crate) fn check_directive<'a>(
     let is_known = |s: &str| {
         KNOWN_DIRECTIVE_NAMES.contains(&s)
             || match mode {
-                Mode::Rustdoc | Mode::RustdocJson => {
+                TestMode::Rustdoc | TestMode::RustdocJson => {
                     original_line.starts_with("//@")
                         && match mode {
-                            Mode::Rustdoc => KNOWN_HTMLDOCCK_DIRECTIVE_NAMES,
-                            Mode::RustdocJson => KNOWN_JSONDOCCK_DIRECTIVE_NAMES,
+                            TestMode::Rustdoc => KNOWN_HTMLDOCCK_DIRECTIVE_NAMES,
+                            TestMode::RustdocJson => KNOWN_JSONDOCCK_DIRECTIVE_NAMES,
                             _ => unreachable!(),
                         }
                         .contains(&s)
@@ -868,7 +868,7 @@ pub(crate) fn check_directive<'a>(
 const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@";
 
 fn iter_directives(
-    mode: Mode,
+    mode: TestMode,
     _suite: &str,
     poisoned: &mut bool,
     testfile: &Utf8Path,
@@ -883,7 +883,7 @@ fn iter_directives(
     // specify them manually in every test file.
     //
     // FIXME(jieyouxu): I feel like there's a better way to do this, leaving for later.
-    if mode == Mode::CoverageRun {
+    if mode == TestMode::CoverageRun {
         let extra_directives: &[&str] = &[
             "needs-profiler-runtime",
             // FIXME(pietroalbini): this test currently does not work on cross-compiled targets
@@ -964,7 +964,7 @@ impl Config {
             ["CHECK", "COM", "NEXT", "SAME", "EMPTY", "NOT", "COUNT", "DAG", "LABEL"];
 
         if let Some(raw) = self.parse_name_value_directive(line, "revisions") {
-            if self.mode == Mode::RunMake {
+            if self.mode == TestMode::RunMake {
                 panic!("`run-make` tests do not support revisions: {}", testfile);
             }
 
@@ -981,7 +981,7 @@ impl Config {
                     );
                 }
 
-                if matches!(self.mode, Mode::Assembly | Mode::Codegen | Mode::MirOpt)
+                if matches!(self.mode, TestMode::Assembly | TestMode::Codegen | TestMode::MirOpt)
                     && FILECHECK_FORBIDDEN_REVISION_NAMES.contains(&revision)
                 {
                     panic!(
@@ -1443,7 +1443,7 @@ pub(crate) fn make_test_description<R: Read>(
     // since we run the pretty printer across all tests by default.
     // If desired, we could add a `should-fail-pretty` annotation.
     let should_panic = match config.mode {
-        crate::common::Pretty => ShouldPanic::No,
+        TestMode::Pretty => ShouldPanic::No,
         _ if should_fail => ShouldPanic::Yes,
         _ => ShouldPanic::No,
     };

src/tools/compiletest/src/directives/tests.rs

@@ -7,7 +7,7 @@ use super::{
     DirectivesCache, EarlyProps, extract_llvm_version, extract_version_range, iter_directives,
     parse_normalize_rule,
 };
-use crate::common::{Config, Debugger, Mode};
+use crate::common::{Config, Debugger, TestMode};
 use crate::executor::{CollectedTestDesc, ShouldPanic};
 
 fn make_test_description<R: Read>(
@@ -785,7 +785,7 @@ fn threads_support() {
 
 fn run_path(poisoned: &mut bool, path: &Utf8Path, buf: &[u8]) {
     let rdr = std::io::Cursor::new(&buf);
-    iter_directives(Mode::Ui, "ui", poisoned, path, rdr, &mut |_| {});
+    iter_directives(TestMode::Ui, "ui", poisoned, path, rdr, &mut |_| {});
 }
 
 #[test]