Remove config cloning in compiletest

src/tools/compiletest/src/cli.rs

@@ -8,9 +8,7 @@ use std::sync::{Arc, OnceLock};
 use camino::{Utf8Path, Utf8PathBuf};
 use clap::Parser;
 
-use crate::common::{
-    CodegenBackend, CompareMode, Config, Debugger, ForcePassMode, TestMode, TestSuite,
-};
+use crate::common::{CodegenBackend, CompareMode, Config, ForcePassMode, TestMode, TestSuite};
 use crate::edition::Edition;
 use crate::{debuggers, directives, early_config_check, run_tests};
 
@@ -200,9 +198,6 @@ struct Args {
     /// Default Rust edition.
     #[arg(long)]
     edition: Option<Edition>,
-    /// Only test a specific debugger in debuginfo tests.
-    #[arg(long)]
-    debugger: Option<String>,
     /// The codegen backend currently used.
     #[arg(long)]
     default_codegen_backend: Option<CodegenBackend>,
@@ -422,11 +417,6 @@ pub(crate) fn parse_config(args: Vec<String>) -> Config {
 
         mode,
         suite: args.suite,
-        debugger: args.debugger.map(|debugger| {
-            debugger
-                .parse::<Debugger>()
-                .unwrap_or_else(|_| panic!("unknown `--debugger` option `{debugger}` given"))
-        }),
         run_ignored: args.ignored,
         with_rustc_debug_assertions: args.with_rustc_debug_assertions,
         with_std_debug_assertions: args.with_std_debug_assertions,

src/tools/compiletest/src/common.rs

@@ -10,6 +10,7 @@ use camino::{Utf8Path, Utf8PathBuf};
 use semver::Version;
 
 use crate::edition::Edition;
+use crate::executor::TestVariant;
 use crate::fatal;
 use crate::util::{Utf8PathBufExt, add_dylib_path, string_enum};
 
@@ -452,18 +453,6 @@ pub(crate) struct Config {
     /// [`TestMode::CoverageMap`].
     pub(crate) suite: TestSuite,
 
-    /// When specified, **only** the specified [`Debugger`] will be used to run against the
-    /// `tests/debuginfo` test suite. When unspecified, `compiletest` will attempt to find all three
-    /// of {`lldb`, `cdb`, `gdb`} implicitly, and then try to run the `debuginfo` test suite against
-    /// all three debuggers.
-    ///
-    /// FIXME: this implicit behavior is really nasty, in that it makes it hard for the user to
-    /// control *which* debugger(s) are available and used to run the debuginfo test suite. We
-    /// should have `bootstrap` allow the user to *explicitly* configure the debuggers, and *not*
-    /// try to implicitly discover some random debugger from the user environment. This makes the
-    /// debuginfo test suite particularly hard to work with.
-    pub(crate) debugger: Option<Debugger>,
-
     /// Run ignored tests *unconditionally*, overriding their ignore reason.
     ///
     /// FIXME: this is wired up through the test execution logic, but **not** accessible from
@@ -1306,13 +1295,13 @@ pub(crate) fn output_relative_path(config: &Config, relative_dir: &Utf8Path) ->
 pub(crate) fn output_testname_unique(
     config: &Config,
     testpaths: &TestPaths,
-    revision: Option<&str>,
+    variant: &TestVariant,
 ) -> Utf8PathBuf {
     let mode = config.compare_mode.as_ref().map_or("", |m| m.to_str());
-    let debugger = config.debugger.as_ref().map_or("", |m| m.to_str());
+    let debugger = variant.debugger.as_ref().map_or("", |m| m.to_str());
     Utf8PathBuf::from(&testpaths.file.file_stem().unwrap())
         .with_extra_extension(config.mode.output_dir_disambiguator())
-        .with_extra_extension(revision.unwrap_or(""))
+        .with_extra_extension(variant.revision().unwrap_or(""))
         .with_extra_extension(mode)
         .with_extra_extension(debugger)
 }
@@ -1323,10 +1312,10 @@ pub(crate) fn output_testname_unique(
 pub(crate) fn output_base_dir(
     config: &Config,
     testpaths: &TestPaths,
-    revision: Option<&str>,
+    variant: &TestVariant,
 ) -> Utf8PathBuf {
     output_relative_path(config, &testpaths.relative_dir)
-        .join(output_testname_unique(config, testpaths, revision))
+        .join(output_testname_unique(config, testpaths, variant))
 }
 
 /// Absolute path to the base filename used as output for the given
@@ -1335,17 +1324,17 @@ pub(crate) fn output_base_dir(
 pub(crate) fn output_base_name(
     config: &Config,
     testpaths: &TestPaths,
-    revision: Option<&str>,
+    variant: &TestVariant,
 ) -> Utf8PathBuf {
-    output_base_dir(config, testpaths, revision).join(testpaths.file.file_stem().unwrap())
+    output_base_dir(config, testpaths, variant).join(testpaths.file.file_stem().unwrap())
 }
 
 /// Absolute path to the directory to use for incremental compilation. Example:
 ///   /path/to/build/host-tuple/test/ui/relative/testname.mode/testname.inc
 pub(crate) fn incremental_dir(
     config: &Config,
     testpaths: &TestPaths,
-    revision: Option<&str>,
+    variant: &TestVariant,
 ) -> Utf8PathBuf {
-    output_base_name(config, testpaths, revision).with_extension("inc")
+    output_base_name(config, testpaths, variant).with_extension("inc")
 }

src/tools/compiletest/src/directives.rs

@@ -20,7 +20,7 @@ use crate::directives::line::DirectiveLine;
 use crate::directives::needs::PreparedNeedsConditions;
 use crate::edition::{Edition, parse_edition};
 use crate::errors::ErrorKind;
-use crate::executor::{CollectedTestDesc, ShouldFail};
+use crate::executor::{CollectedTestDesc, ShouldFail, TestVariant};
 use crate::util::static_regex;
 use crate::{fatal, help};
 
@@ -892,7 +892,7 @@ pub(crate) fn make_test_description(
     path: &Utf8Path,
     filterable_path: &Utf8Path,
     file_directives: &FileDirectives<'_>,
-    test_revision: Option<&str>,
+    variant: &TestVariant,
     poisoned: &mut bool,
     aux_props: &mut AuxProps,
 ) -> CollectedTestDesc {
@@ -901,25 +901,25 @@ pub(crate) fn make_test_description(
 
     // Perform a per-file (rather than per-line) ignore decision to skip running debuginfo tests
     // if we don't have a debugger for them available.
-    // This is needed because we duplicate the Config once for each debugger.
-    if config.mode == TestMode::DebugInfo {
-        match &config.debugger {
-            Some(Debugger::Cdb) => {
+    // We do this to materialize debuginfo tests for each debugger and explicitly ignore
+    // the variants that are not supported in our environment.
+    if let Some(debugger) = variant.debugger.as_ref() {
+        match debugger {
+            Debugger::Cdb => {
                 if let Some(msg) = check_cdb_support(config) {
                     ignore_message = Some(Cow::Owned(msg));
                 }
             }
-            Some(Debugger::Gdb) => {
+            Debugger::Gdb => {
                 if let Some(msg) = check_gdb_support(config) {
                     ignore_message = Some(Cow::Owned(msg));
                 }
             }
-            Some(Debugger::Lldb) => {
+            Debugger::Lldb => {
                 if let Some(msg) = check_lldb_support(config) {
                     ignore_message = Some(Cow::Owned(msg));
                 }
             }
-            None => {}
         }
     }
 
@@ -929,7 +929,7 @@ pub(crate) fn make_test_description(
             config,
             file_directives,
             &mut |ln @ &DirectiveLine { line_number, .. }| {
-                if !ln.applies_to_test_revision(test_revision) {
+                if !ln.applies_to_test_revision(variant.revision()) {
                     return;
                 }
 
@@ -958,9 +958,9 @@ pub(crate) fn make_test_description(
                 decision!(ignore_llvm(config, ln));
                 decision!(ignore_backends(config, ln));
                 decision!(needs_backends(config, ln));
-                decision!(ignore_cdb(config, ln));
-                decision!(ignore_gdb(config, ln));
-                decision!(ignore_lldb(config, ln));
+                decision!(ignore_cdb(config, variant, ln));
+                decision!(ignore_gdb(config, variant, ln));
+                decision!(ignore_lldb(config, variant, ln));
                 decision!(ignore_parallel_frontend(config, ln));
 
                 if config.target == "wasm32-unknown-unknown"
@@ -1019,9 +1019,17 @@ fn check_lldb_support(config: &Config) -> Option<String> {
     if config.lldb.is_none() { Some("lldb is not available".to_string()) } else { None }
 }
 
-fn ignore_cdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision {
-    if config.debugger != Some(Debugger::Cdb) {
-        return IgnoreDecision::Continue;
+fn ignore_cdb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) -> IgnoreDecision {
+    if variant.debugger != Some(Debugger::Cdb) {
+        return if line.name == "only-cdb" {
+            IgnoreDecision::Ignore { reason: "debugger is not cdb".to_string() }
+        } else {
+            IgnoreDecision::Continue
+        };
+    }
+
+    if line.name == "ignore-cdb" {
+        return IgnoreDecision::Ignore { reason: "debugger is cdb".to_string() };
     }
 
     if let Some(actual_version) = config.cdb_version {
@@ -1044,9 +1052,17 @@ fn ignore_cdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision {
     IgnoreDecision::Continue
 }
 
-fn ignore_gdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision {
-    if config.debugger != Some(Debugger::Gdb) {
-        return IgnoreDecision::Continue;
+fn ignore_gdb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) -> IgnoreDecision {
+    if variant.debugger != Some(Debugger::Gdb) {
+        return if line.name == "only-gdb" {
+            IgnoreDecision::Ignore { reason: "debugger is not gdb".to_string() }
+        } else {
+            IgnoreDecision::Continue
+        };
+    }
+
+    if line.name == "ignore-gdb" {
+        return IgnoreDecision::Ignore { reason: "debugger is gdb".to_string() };
     }
 
     if let Some(actual_version) = config.gdb_version {
@@ -1096,9 +1112,17 @@ fn ignore_gdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision {
     IgnoreDecision::Continue
 }
 
-fn ignore_lldb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision {
-    if config.debugger != Some(Debugger::Lldb) {
-        return IgnoreDecision::Continue;
+fn ignore_lldb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) -> IgnoreDecision {
+    if variant.debugger != Some(Debugger::Lldb) {
+        return if line.name == "only-lldb" {
+            IgnoreDecision::Ignore { reason: "debugger is not lldb".to_string() }
+        } else {
+            IgnoreDecision::Continue
+        };
+    }
+
+    if line.name == "ignore-lldb" {
+        return IgnoreDecision::Ignore { reason: "debugger is lldb".to_string() };
     }
 
     if let Some(actual_version) = config.lldb_version {

src/tools/compiletest/src/directives/cfg.rs

@@ -1,25 +1,41 @@
 use std::collections::{HashMap, HashSet};
 use std::sync::{Arc, LazyLock};
 
-use crate::common::{CompareMode, Config, Debugger};
+use crate::common::{CompareMode, Config};
 use crate::directives::{DirectiveLine, IgnoreDecision};
 
 const EXTRA_ARCHS: &[&str] = &["spirv"];
 
 const EXTERNAL_IGNORES_LIST: &[&str] = &[
     // tidy-alphabetical-start
     "ignore-backends",
+    "ignore-cdb",
+    "ignore-gdb",
     "ignore-gdb-version",
+    "ignore-lldb",
     "ignore-llvm-version",
     "ignore-parallel-frontend",
     // tidy-alphabetical-end
 ];
 
+const EXTERNAL_ONLY_LIST: &[&str] = &[
+    // tidy-alphabetical-start
+    "only-cdb",
+    "only-gdb",
+    "only-lldb",
+    // tidy-alphabetical-end
+];
+
 /// Directive names that begin with `ignore-`, but are disregarded by this
 /// module because they are handled elsewhere.
 pub(crate) static EXTERNAL_IGNORES_SET: LazyLock<HashSet<&str>> =
     LazyLock::new(|| EXTERNAL_IGNORES_LIST.iter().copied().collect());
 
+/// Directive names that begin with `only-`, but are disregarded by this
+/// module because they are handled elsewhere.
+pub(crate) static EXTERNAL_ONLY_SET: LazyLock<HashSet<&str>> =
+    LazyLock::new(|| EXTERNAL_ONLY_LIST.iter().copied().collect());
+
 pub(super) fn handle_ignore(
     conditions: &PreparedConditions,
     line: &DirectiveLine<'_>,
@@ -75,6 +91,8 @@ fn parse_cfg_name_directive<'a>(
 
     if prefix == "ignore-" && EXTERNAL_IGNORES_SET.contains(line.name) {
         return ParsedNameDirective::not_handled_here();
+    } else if prefix == "only-" && EXTERNAL_ONLY_SET.contains(line.name) {
+        return ParsedNameDirective::not_handled_here();
     }
 
     // FIXME(Zalathar): This currently allows either a space or a colon, and
@@ -209,14 +227,6 @@ pub(crate) fn prepare_conditions(config: &Config) -> PreparedConditions {
         "when std is built with remapping of debuginfo",
     );
 
-    for &debugger in Debugger::STR_VARIANTS {
-        builder.cond(
-            debugger,
-            Some(debugger) == config.debugger.as_ref().map(Debugger::to_str),
-            &format!("when the debugger is {debugger}"),
-        );
-    }
-
     for &compare_mode in CompareMode::STR_VARIANTS {
         builder.cond(
             &format!("compare-mode-{compare_mode}"),

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

@@ -9,7 +9,7 @@ use crate::directives::{
     FileDirectives, KNOWN_DIRECTIVE_NAMES_SET, LineNumber, extract_llvm_version,
     extract_version_range, line_directive, parse_edition, parse_normalize_rule,
 };
-use crate::executor::{CollectedTestDesc, ShouldFail};
+use crate::executor::{CollectedTestDesc, ShouldFail, TestVariant};
 
 /// All directive handlers should have a name that is also in `KNOWN_DIRECTIVE_NAMES_SET`.
 #[test]
@@ -46,11 +46,14 @@ fn make_test_description(
     filterable_path: &Utf8Path,
     file_contents: &str,
     revision: Option<&str>,
+    debugger: Option<Debugger>,
 ) -> CollectedTestDesc {
     let cache = DirectivesCache::load(config);
     let mut poisoned = false;
     let file_directives = FileDirectives::from_file_contents(path, file_contents);
 
+    let variant = TestVariant { revision: revision.map(str::to_owned), debugger };
+
     let mut aux_props = AuxProps::default();
     let test = crate::directives::make_test_description(
         config,
@@ -59,7 +62,7 @@ fn make_test_description(
         path,
         filterable_path,
         &file_directives,
-        revision,
+        &variant,
         &mut poisoned,
         &mut aux_props,
     );
@@ -283,7 +286,14 @@ fn parse_early_props(config: &Config, contents: &str) -> EarlyProps {
 fn check_ignore(config: &Config, contents: &str) -> bool {
     let tn = String::new();
     let p = Utf8Path::new("a.rs");
-    let d = make_test_description(&config, tn, p, p, contents, None);
+    let d = make_test_description(&config, tn, p, p, contents, None, None);
+    d.is_ignored()
+}
+
+fn check_ignore_debugger(config: &Config, contents: &str, debugger: Option<Debugger>) -> bool {
+    let tn = String::new();
+    let p = Utf8Path::new("a.rs");
+    let d = make_test_description(&config, tn, p, p, contents, None, debugger);
     d.is_ignored()
 }
 
@@ -293,9 +303,9 @@ fn should_fail() {
     let tn = String::new();
     let p = Utf8Path::new("a.rs");
 
-    let d = make_test_description(&config, tn.clone(), p, p, "", None);
+    let d = make_test_description(&config, tn.clone(), p, p, "", None, None);
     assert_eq!(d.should_fail, ShouldFail::No);
-    let d = make_test_description(&config, tn, p, p, "//@ should-fail", None);
+    let d = make_test_description(&config, tn, p, p, "//@ should-fail", None, None);
     assert_eq!(d.should_fail, ShouldFail::Yes);
 }
 
@@ -449,18 +459,11 @@ fn cross_compile() {
 
 #[test]
 fn debugger() {
-    let mut config = cfg().build();
-    config.debugger = None;
-    assert!(!check_ignore(&config, "//@ ignore-cdb"));
-
-    config.debugger = Some(Debugger::Cdb);
-    assert!(check_ignore(&config, "//@ ignore-cdb"));
-
-    config.debugger = Some(Debugger::Gdb);
-    assert!(check_ignore(&config, "//@ ignore-gdb"));
-
-    config.debugger = Some(Debugger::Lldb);
-    assert!(check_ignore(&config, "//@ ignore-lldb"));
+    let config = cfg().build();
+    assert!(!check_ignore_debugger(&config, "//@ ignore-cdb", None));
+    assert!(check_ignore_debugger(&config, "//@ ignore-cdb", Some(Debugger::Cdb)));
+    assert!(check_ignore_debugger(&config, "//@ ignore-gdb", Some(Debugger::Gdb)));
+    assert!(check_ignore_debugger(&config, "//@ ignore-lldb", Some(Debugger::Lldb)));
 }
 
 #[test]