From 6250d772178cd7fbd028cb22d11bff211cb74870 Mon Sep 17 00:00:00 2001 From: binarycat Date: Fri, 17 Jan 2025 13:50:32 -0600 Subject: [PATCH 1/2] factor out enums from compiletest into build_helper::compiletest --- src/build_helper/src/compiletest.rs | 129 ++++++++++++++++++++++++++++ src/build_helper/src/lib.rs | 3 + src/build_helper/src/tests.rs | 26 ++++++ src/tools/compiletest/src/common.rs | 128 +-------------------------- src/tools/compiletest/src/tests.rs | 26 ------ 5 files changed, 160 insertions(+), 152 deletions(-) create mode 100644 src/build_helper/src/compiletest.rs create mode 100644 src/build_helper/src/tests.rs diff --git a/src/build_helper/src/compiletest.rs b/src/build_helper/src/compiletest.rs new file mode 100644 index 0000000000000..e6031f3861ee6 --- /dev/null +++ b/src/build_helper/src/compiletest.rs @@ -0,0 +1,129 @@ +//! Types representing arguments to compiletest. + +use std::str::FromStr; +use std::fmt; +pub use self::Mode::*; + +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 { + 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; + +string_enum! { + #[derive(Clone, Copy, PartialEq, Debug)] + pub enum Mode { + Pretty => "pretty", + DebugInfo => "debuginfo", + Codegen => "codegen", + Rustdoc => "rustdoc", + RustdocJson => "rustdoc-json", + CodegenUnits => "codegen-units", + Incremental => "incremental", + RunMake => "run-make", + Ui => "ui", + RustdocJs => "rustdoc-js", + MirOpt => "mir-opt", + Assembly => "assembly", + CoverageMap => "coverage-map", + CoverageRun => "coverage-run", + Crashes => "crashes", + } +} + +impl Default for Mode { + fn default() -> Self { + Mode::Ui + } +} + +impl Mode { + 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", + _ => "", + } + } + + pub fn output_dir_disambiguator(self) -> &'static str { + // 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(), + _ => "", + } + } +} + +string_enum! { + #[derive(Clone, Copy, PartialEq, Debug, Hash)] + pub enum PassMode { + Check => "check", + Build => "build", + Run => "run", + } +} + +#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)] +pub enum FailMode { + Check, + Build, + Run, +} + +string_enum! { + #[derive(Clone, Debug, PartialEq)] + pub enum CompareMode { + Polonius => "polonius", + NextSolver => "next-solver", + NextSolverCoherence => "next-solver-coherence", + SplitDwarf => "split-dwarf", + SplitDwarfSingle => "split-dwarf-single", + } +} + +string_enum! { + #[derive(Clone, Copy, Debug, PartialEq)] + pub enum Debugger { + Cdb => "cdb", + Gdb => "gdb", + Lldb => "lldb", + } +} diff --git a/src/build_helper/src/lib.rs b/src/build_helper/src/lib.rs index dceb5fdeeea5c..4f22754d73e81 100644 --- a/src/build_helper/src/lib.rs +++ b/src/build_helper/src/lib.rs @@ -7,6 +7,9 @@ pub mod git; pub mod metrics; pub mod stage0_parser; pub mod util; +pub mod compiletest; +#[cfg(test)] +mod tests; /// The default set of crates for opt-dist to collect LLVM profiles. pub const LLVM_PGO_CRATES: &[&str] = &[ diff --git a/src/build_helper/src/tests.rs b/src/build_helper/src/tests.rs new file mode 100644 index 0000000000000..60ea1a3dac823 --- /dev/null +++ b/src/build_helper/src/tests.rs @@ -0,0 +1,26 @@ +#[test] +fn string_enums() { + // These imports are needed for the macro-generated code + use std::fmt; + use std::str::FromStr; + + crate::compiletest::string_enum! { + #[derive(Clone, Copy, Debug, PartialEq)] + enum Animal { + Cat => "meow", + Dog => "woof", + } + } + + // General assertions, mostly to silence the dead code warnings + assert_eq!(Animal::VARIANTS.len(), 2); + assert_eq!(Animal::STR_VARIANTS.len(), 2); + + // Correct string conversions + assert_eq!(Animal::Cat, "meow".parse().unwrap()); + assert_eq!(Animal::Dog, "woof".parse().unwrap()); + + // Invalid conversions + let animal = "nya".parse::(); + assert_eq!("unknown `Animal` variant: `nya`", animal.unwrap_err()); +} diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index c6f3d7c0d1080..7fea92dd1dad2 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -2,141 +2,17 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::ffi::OsString; use std::path::{Path, PathBuf}; use std::process::Command; -use std::str::FromStr; use std::sync::OnceLock; -use std::{fmt, iter}; +use std::iter; use build_helper::git::GitConfig; use semver::Version; use serde::de::{Deserialize, Deserializer, Error as _}; use test::{ColorConfig, OutputFormat}; -pub use self::Mode::*; use crate::util::{PathBufExt, add_dylib_path}; +pub use build_helper::compiletest::*; -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 { - 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; - -string_enum! { - #[derive(Clone, Copy, PartialEq, Debug)] - pub enum Mode { - Pretty => "pretty", - DebugInfo => "debuginfo", - Codegen => "codegen", - Rustdoc => "rustdoc", - RustdocJson => "rustdoc-json", - CodegenUnits => "codegen-units", - Incremental => "incremental", - RunMake => "run-make", - Ui => "ui", - RustdocJs => "rustdoc-js", - MirOpt => "mir-opt", - Assembly => "assembly", - CoverageMap => "coverage-map", - CoverageRun => "coverage-run", - Crashes => "crashes", - } -} - -impl Default for Mode { - fn default() -> Self { - Mode::Ui - } -} - -impl Mode { - 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", - _ => "", - } - } - - pub fn output_dir_disambiguator(self) -> &'static str { - // 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(), - _ => "", - } - } -} - -string_enum! { - #[derive(Clone, Copy, PartialEq, Debug, Hash)] - pub enum PassMode { - Check => "check", - Build => "build", - Run => "run", - } -} - -#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)] -pub enum FailMode { - Check, - Build, - Run, -} - -string_enum! { - #[derive(Clone, Debug, PartialEq)] - pub enum CompareMode { - Polonius => "polonius", - NextSolver => "next-solver", - NextSolverCoherence => "next-solver-coherence", - SplitDwarf => "split-dwarf", - SplitDwarfSingle => "split-dwarf-single", - } -} - -string_enum! { - #[derive(Clone, Copy, Debug, PartialEq)] - pub enum Debugger { - Cdb => "cdb", - Gdb => "gdb", - Lldb => "lldb", - } -} #[derive(Clone, Copy, Debug, PartialEq, Default, serde::Deserialize)] #[serde(rename_all = "kebab-case")] diff --git a/src/tools/compiletest/src/tests.rs b/src/tools/compiletest/src/tests.rs index 43c6dc0a67e89..75819865f8853 100644 --- a/src/tools/compiletest/src/tests.rs +++ b/src/tools/compiletest/src/tests.rs @@ -67,29 +67,3 @@ fn is_test_test() { assert!(!is_test(&OsString::from("~a_temp_file"))); } -#[test] -fn string_enums() { - // These imports are needed for the macro-generated code - use std::fmt; - use std::str::FromStr; - - crate::common::string_enum! { - #[derive(Clone, Copy, Debug, PartialEq)] - enum Animal { - Cat => "meow", - Dog => "woof", - } - } - - // General assertions, mostly to silence the dead code warnings - assert_eq!(Animal::VARIANTS.len(), 2); - assert_eq!(Animal::STR_VARIANTS.len(), 2); - - // Correct string conversions - assert_eq!(Animal::Cat, "meow".parse().unwrap()); - assert_eq!(Animal::Dog, "woof".parse().unwrap()); - - // Invalid conversions - let animal = "nya".parse::(); - assert_eq!("unknown `Animal` variant: `nya`", animal.unwrap_err()); -} From 70eac071d669106c91e4dcb6bc9c1142793d6cba Mon Sep 17 00:00:00 2001 From: binarycat Date: Fri, 17 Jan 2025 14:16:29 -0600 Subject: [PATCH 2/2] bootstrap now uses enum for compiletest modes --- src/bootstrap/src/core/build_steps/test.rs | 68 ++++++++++++---------- src/build_helper/src/compiletest.rs | 5 +- src/build_helper/src/lib.rs | 4 +- src/tools/compiletest/src/common.rs | 5 +- src/tools/compiletest/src/tests.rs | 1 - 5 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 9f3e4d9cc8995..7a9532b58c810 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -8,6 +8,7 @@ use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; use std::{env, fs, iter}; +use build_helper::compiletest::Mode as CtMode; use clap_complete::shells; use crate::core::build_steps::compile::run_cargo; @@ -915,7 +916,7 @@ impl Step for RustdocJSNotStd { builder.ensure(Compiletest { compiler: self.compiler, target: self.target, - mode: "rustdoc-js", + mode: CtMode::RustdocJs, suite: "rustdoc-js", path: "tests/rustdoc-js", compare_mode: None, @@ -1347,29 +1348,29 @@ impl Step for CrateBuildHelper { } } -test!(Ui { path: "tests/ui", mode: "ui", suite: "ui", default: true }); +test!(Ui { path: "tests/ui", mode: CtMode::Ui, suite: "ui", default: true }); -test!(Crashes { path: "tests/crashes", mode: "crashes", suite: "crashes", default: true }); +test!(Crashes { path: "tests/crashes", mode: CtMode::Crashes, suite: "crashes", default: true }); -test!(Codegen { path: "tests/codegen", mode: "codegen", suite: "codegen", default: true }); +test!(Codegen { path: "tests/codegen", mode: CtMode::Codegen, suite: "codegen", default: true }); test!(CodegenUnits { path: "tests/codegen-units", - mode: "codegen-units", + mode: CtMode::CodegenUnits, suite: "codegen-units", default: true, }); test!(Incremental { path: "tests/incremental", - mode: "incremental", + mode: CtMode::Incremental, suite: "incremental", default: true, }); test!(Debuginfo { path: "tests/debuginfo", - mode: "debuginfo", + mode: CtMode::DebugInfo, suite: "debuginfo", default: true, compare_mode: Some("split-dwarf"), @@ -1377,7 +1378,7 @@ test!(Debuginfo { test!(UiFullDeps { path: "tests/ui-fulldeps", - mode: "ui", + mode: CtMode::Ui, suite: "ui-fulldeps", default: true, only_hosts: true, @@ -1385,14 +1386,14 @@ test!(UiFullDeps { test!(Rustdoc { path: "tests/rustdoc", - mode: "rustdoc", + mode: CtMode::Rustdoc, suite: "rustdoc", default: true, only_hosts: true, }); test!(RustdocUi { path: "tests/rustdoc-ui", - mode: "ui", + mode: CtMode::Ui, suite: "rustdoc-ui", default: true, only_hosts: true, @@ -1400,7 +1401,7 @@ test!(RustdocUi { test!(RustdocJson { path: "tests/rustdoc-json", - mode: "rustdoc-json", + mode: CtMode::RustdocJson, suite: "rustdoc-json", default: true, only_hosts: true, @@ -1408,7 +1409,7 @@ test!(RustdocJson { test!(Pretty { path: "tests/pretty", - mode: "pretty", + mode: CtMode::Pretty, suite: "pretty", default: true, only_hosts: true, @@ -1441,7 +1442,7 @@ impl Step for RunMake { builder.ensure(Compiletest { compiler: self.compiler, target: self.target, - mode: "run-make", + mode: CtMode::RunMake, suite: "run-make", path: "tests/run-make", compare_mode: None, @@ -1449,7 +1450,12 @@ impl Step for RunMake { } } -test!(Assembly { path: "tests/assembly", mode: "assembly", suite: "assembly", default: true }); +test!(Assembly { + path: "tests/assembly", + mode: CtMode::Assembly, + suite: "assembly", + default: true +}); /// Runs the coverage test suite at `tests/coverage` in some or all of the /// coverage test modes. @@ -1536,7 +1542,7 @@ impl Step for Coverage { builder.ensure(Compiletest { compiler, target, - mode, + mode: mode.parse().unwrap(), suite: Self::SUITE, path: Self::PATH, compare_mode: None, @@ -1546,7 +1552,7 @@ impl Step for Coverage { test!(CoverageRunRustdoc { path: "tests/coverage-run-rustdoc", - mode: "coverage-run", + mode: CtMode::CoverageRun, suite: "coverage-run-rustdoc", default: true, only_hosts: true, @@ -1578,7 +1584,7 @@ impl Step for MirOpt { builder.ensure(Compiletest { compiler: self.compiler, target, - mode: "mir-opt", + mode: CtMode::MirOpt, suite: "mir-opt", path: "tests/mir-opt", compare_mode: None, @@ -1615,7 +1621,7 @@ impl Step for MirOpt { struct Compiletest { compiler: Compiler, target: TargetSelection, - mode: &'static str, + mode: CtMode, suite: &'static str, path: &'static str, compare_mode: Option<&'static str>, @@ -1729,7 +1735,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the let is_rustdoc = suite == "rustdoc-ui" || suite == "rustdoc-js"; - if mode == "run-make" { + if mode == CtMode::RunMake { let cargo_path = if builder.top_stage == 0 { // If we're using `--stage 0`, we should provide the bootstrap cargo. builder.initial_cargo.clone() @@ -1752,17 +1758,17 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the } // Avoid depending on rustdoc when we don't need it. - if mode == "rustdoc" - || mode == "run-make" - || (mode == "ui" && is_rustdoc) - || mode == "rustdoc-js" - || mode == "rustdoc-json" + if mode == CtMode::Rustdoc + || mode == CtMode::RunMake + || (mode == CtMode::Ui && is_rustdoc) + || mode == CtMode::RustdocJs + || mode == CtMode::RustdocJson || suite == "coverage-run-rustdoc" { cmd.arg("--rustdoc-path").arg(builder.rustdoc(compiler)); } - if mode == "rustdoc-json" { + if mode == CtMode::RustdocJson { // Use the beta compiler for jsondocck let json_compiler = compiler.with_stage(0); cmd.arg("--jsondocck-path") @@ -1771,7 +1777,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the .arg(builder.ensure(tool::JsonDocLint { compiler: json_compiler, target })); } - if matches!(mode, "coverage-map" | "coverage-run") { + if matches!(mode, CtMode::CoverageMap | CtMode::CoverageRun) { let coverage_dump = builder.tool_exe(Tool::CoverageDump); cmd.arg("--coverage-dump-path").arg(coverage_dump); } @@ -1789,7 +1795,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the cmd.arg("--sysroot-base").arg(sysroot); cmd.arg("--stage-id").arg(stage_id); cmd.arg("--suite").arg(suite); - cmd.arg("--mode").arg(mode); + cmd.arg("--mode").arg(mode.to_str()); cmd.arg("--target").arg(target.rustc_target_arg()); cmd.arg("--host").arg(&*compiler.host.triple); cmd.arg("--llvm-filecheck").arg(builder.llvm_filecheck(builder.config.build)); @@ -1827,7 +1833,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the if let Some(ref nodejs) = builder.config.nodejs { cmd.arg("--nodejs").arg(nodejs); - } else if mode == "rustdoc-js" { + } else if mode == CtMode::RustdocJs { panic!("need nodejs to run rustdoc-js suite"); } if let Some(ref npm) = builder.config.npm { @@ -1985,7 +1991,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the cmd.env("RUSTFLAGS", rustflags); } - if !builder.config.dry_run() && matches!(mode, "run-make" | "coverage-run") { + if !builder.config.dry_run() && matches!(mode, CtMode::RunMake | CtMode::CoverageRun) { // The llvm/bin directory contains many useful cross-platform // tools. Pass the path to run-make tests so they can use them. // (The coverage-run tests also need these tools to process @@ -1997,7 +2003,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the cmd.arg("--llvm-bin-dir").arg(llvm_bin_path); } - if !builder.config.dry_run() && mode == "run-make" { + if !builder.config.dry_run() && mode == CtMode::RunMake { // If LLD is available, add it to the PATH if builder.config.lld_enabled { let lld_install_root = @@ -2017,7 +2023,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the // Only pass correct values for these flags for the `run-make` suite as it // requires that a C++ compiler was configured which isn't always the case. - if !builder.config.dry_run() && mode == "run-make" { + if !builder.config.dry_run() && mode == CtMode::RunMake { cmd.arg("--cc") .arg(builder.cc(target)) .arg("--cxx") diff --git a/src/build_helper/src/compiletest.rs b/src/build_helper/src/compiletest.rs index e6031f3861ee6..2f0f5fce92c8b 100644 --- a/src/build_helper/src/compiletest.rs +++ b/src/build_helper/src/compiletest.rs @@ -1,7 +1,8 @@ //! Types representing arguments to compiletest. -use std::str::FromStr; use std::fmt; +use std::str::FromStr; + pub use self::Mode::*; macro_rules! string_enum { @@ -46,7 +47,7 @@ macro_rules! string_enum { pub(crate) use string_enum; string_enum! { - #[derive(Clone, Copy, PartialEq, Debug)] + #[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)] pub enum Mode { Pretty => "pretty", DebugInfo => "debuginfo", diff --git a/src/build_helper/src/lib.rs b/src/build_helper/src/lib.rs index 4f22754d73e81..88a2ff758f47b 100644 --- a/src/build_helper/src/lib.rs +++ b/src/build_helper/src/lib.rs @@ -1,15 +1,15 @@ //! Types and functions shared across tools in this workspace. pub mod ci; +pub mod compiletest; pub mod drop_bomb; pub mod fs; pub mod git; pub mod metrics; pub mod stage0_parser; -pub mod util; -pub mod compiletest; #[cfg(test)] mod tests; +pub mod util; /// The default set of crates for opt-dist to collect LLVM profiles. pub const LLVM_PGO_CRATES: &[&str] = &[ diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 7fea92dd1dad2..2e6aa312828c9 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -1,18 +1,17 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::ffi::OsString; +use std::iter; use std::path::{Path, PathBuf}; use std::process::Command; use std::sync::OnceLock; -use std::iter; +pub use build_helper::compiletest::*; use build_helper::git::GitConfig; use semver::Version; use serde::de::{Deserialize, Deserializer, Error as _}; use test::{ColorConfig, OutputFormat}; use crate::util::{PathBufExt, add_dylib_path}; -pub use build_helper::compiletest::*; - #[derive(Clone, Copy, Debug, PartialEq, Default, serde::Deserialize)] #[serde(rename_all = "kebab-case")] diff --git a/src/tools/compiletest/src/tests.rs b/src/tools/compiletest/src/tests.rs index 75819865f8853..fec746904dedf 100644 --- a/src/tools/compiletest/src/tests.rs +++ b/src/tools/compiletest/src/tests.rs @@ -66,4 +66,3 @@ fn is_test_test() { assert!(!is_test(&OsString::from("#a_dog_gif"))); assert!(!is_test(&OsString::from("~a_temp_file"))); } -