From 9971008b8d83380797df2ab586fcfcfb04f4cfb9 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Thu, 14 Sep 2023 21:03:30 +0300 Subject: [PATCH] micro-level optimizations for bootstrap Overall optimizations for bootstrap on conditions, assertions, trait implementations, etc. Signed-off-by: onur-ozkan --- src/bootstrap/builder.rs | 2 +- src/bootstrap/compile.rs | 6 ++---- src/bootstrap/config.rs | 29 +++++++---------------------- src/bootstrap/config/tests.rs | 14 +++++++------- src/bootstrap/download.rs | 2 +- src/bootstrap/lib.rs | 11 ++++++----- src/bootstrap/llvm.rs | 32 +++++++++++++++++--------------- src/bootstrap/sanity.rs | 21 ++++++++++----------- src/bootstrap/test.rs | 20 +++++++++----------- 9 files changed, 60 insertions(+), 77 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index b3666192853..7e0f3ccae0b 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -523,7 +523,7 @@ pub fn paths(mut self, paths: &[&str]) -> Self { .iter() .map(|p| { // assert only if `p` isn't submodule - if !submodules_paths.iter().find(|sm_p| p.contains(*sm_p)).is_some() { + if submodules_paths.iter().find(|sm_p| p.contains(*sm_p)).is_none() { assert!( self.builder.src.join(p).exists(), "`should_run.paths` should correspond to real on-disk paths - use `alias` if there is no relevant path: {}", diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 9c68e5a78d8..2686a8c1752 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -876,10 +876,8 @@ fn run(self, builder: &Builder<'_>) { cargo.rustflag("-Clto=off"); } } - } else { - if builder.config.rust_lto == RustcLto::Off { - cargo.rustflag("-Clto=off"); - } + } else if builder.config.rust_lto == RustcLto::Off { + cargo.rustflag("-Clto=off"); } for krate in &*self.crates { diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index e5fdac3ceda..176ef8e92db 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -322,33 +322,23 @@ pub struct RustfmtMetadata { pub version: String, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub enum RustfmtState { SystemToolchain(PathBuf), Downloaded(PathBuf), Unavailable, + #[default] LazyEvaluated, } -impl Default for RustfmtState { - fn default() -> Self { - RustfmtState::LazyEvaluated - } -} - -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Default, Clone, Copy, PartialEq)] pub enum LlvmLibunwind { + #[default] No, InTree, System, } -impl Default for LlvmLibunwind { - fn default() -> Self { - Self::No - } -} - impl FromStr for LlvmLibunwind { type Err = String; @@ -362,19 +352,14 @@ fn from_str(value: &str) -> Result { } } -#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Default, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum SplitDebuginfo { Packed, Unpacked, + #[default] Off, } -impl Default for SplitDebuginfo { - fn default() -> Self { - SplitDebuginfo::Off - } -} - impl std::str::FromStr for SplitDebuginfo { type Err = (); @@ -1529,7 +1514,7 @@ fn get_table(option: &str) -> Result { let asserts = llvm_assertions.unwrap_or(false); config.llvm_from_ci = match llvm.download_ci_llvm { Some(StringOrBool::String(s)) => { - assert!(s == "if-available", "unknown option `{s}` for download-ci-llvm"); + assert_eq!(s, "if-available", "unknown option `{s}` for download-ci-llvm"); crate::llvm::is_ci_llvm_available(&config, asserts) } Some(StringOrBool::Bool(b)) => b, diff --git a/src/bootstrap/config/tests.rs b/src/bootstrap/config/tests.rs index b8f3be96062..aac76cdcbcf 100644 --- a/src/bootstrap/config/tests.rs +++ b/src/bootstrap/config/tests.rs @@ -136,7 +136,7 @@ fn override_toml() { "setting string value without quotes" ); assert_eq!(config.gdb, Some("bar".into()), "setting string value with quotes"); - assert_eq!(config.deny_warnings, false, "setting boolean value"); + assert!(!config.deny_warnings, "setting boolean value"); assert_eq!( config.tools, Some(["cargo".to_string()].into_iter().collect()), @@ -181,13 +181,13 @@ fn get_toml(file: &Path) -> TomlConfig { #[test] fn rust_optimize() { - assert_eq!(parse("").rust_optimize.is_release(), true); - assert_eq!(parse("rust.optimize = false").rust_optimize.is_release(), false); - assert_eq!(parse("rust.optimize = true").rust_optimize.is_release(), true); - assert_eq!(parse("rust.optimize = 0").rust_optimize.is_release(), false); - assert_eq!(parse("rust.optimize = 1").rust_optimize.is_release(), true); + assert!(parse("").rust_optimize.is_release()); + assert!(!parse("rust.optimize = false").rust_optimize.is_release()); + assert!(parse("rust.optimize = true").rust_optimize.is_release()); + assert!(!parse("rust.optimize = 0").rust_optimize.is_release()); + assert!(parse("rust.optimize = 1").rust_optimize.is_release()); + assert!(parse("rust.optimize = \"s\"").rust_optimize.is_release()); assert_eq!(parse("rust.optimize = 1").rust_optimize.get_opt_level(), Some("1".to_string())); - assert_eq!(parse("rust.optimize = \"s\"").rust_optimize.is_release(), true); assert_eq!(parse("rust.optimize = \"s\"").rust_optimize.get_opt_level(), Some("s".to_string())); } diff --git a/src/bootstrap/download.rs b/src/bootstrap/download.rs index 17c308e915b..80845af3b9a 100644 --- a/src/bootstrap/download.rs +++ b/src/bootstrap/download.rs @@ -441,7 +441,7 @@ pub(crate) fn download_ci_rustc(&self, commit: &str) { } pub(crate) fn download_beta_toolchain(&self) { - self.verbose(&format!("downloading stage0 beta artifacts")); + self.verbose("downloading stage0 beta artifacts"); let date = &self.stage0_metadata.compiler.date; let version = &self.stage0_metadata.compiler.version; diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 4396bbc51a3..644f61549b8 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -116,7 +116,7 @@ pub unsafe fn setup(_build: &mut crate::Build) {} /// Extra --check-cfg to add when building /// (Mode restriction, config name, config values (if any)) -const EXTRA_CHECK_CFGS: &[(Option, &'static str, Option<&[&'static str]>)] = &[ +const EXTRA_CHECK_CFGS: &[(Option, &str, Option<&[&'static str]>)] = &[ (None, "bootstrap", None), (Some(Mode::Rustc), "parallel_compiler", None), (Some(Mode::ToolRustc), "parallel_compiler", None), @@ -1757,10 +1757,11 @@ fn ninja(&self) -> bool { // // In these cases we automatically enable Ninja if we find it in the // environment. - if !self.config.ninja_in_file && self.config.build.contains("msvc") { - if cmd_finder.maybe_have("ninja").is_some() { - return true; - } + if !self.config.ninja_in_file + && self.config.build.contains("msvc") + && cmd_finder.maybe_have("ninja").is_some() + { + return true; } self.config.ninja_in_file diff --git a/src/bootstrap/llvm.rs b/src/bootstrap/llvm.rs index c841cb34036..ab2967bb8cc 100644 --- a/src/bootstrap/llvm.rs +++ b/src/bootstrap/llvm.rs @@ -148,7 +148,7 @@ pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String { "".to_owned() }; - if &llvm_sha == "" { + if llvm_sha.is_empty() { eprintln!("error: could not find commit hash for downloading LLVM"); eprintln!("help: maybe your repository history is too shallow?"); eprintln!("help: consider disabling `download-ci-llvm`"); @@ -201,10 +201,10 @@ pub(crate) fn is_ci_llvm_available(config: &Config, asserts: bool) -> bool { ("x86_64-unknown-netbsd", false), ]; - if !supported_platforms.contains(&(&*config.build.triple, asserts)) { - if asserts == true || !supported_platforms.contains(&(&*config.build.triple, true)) { - return false; - } + if !supported_platforms.contains(&(&*config.build.triple, asserts)) + && (asserts || !supported_platforms.contains(&(&*config.build.triple, true))) + { + return false; } if is_ci_llvm_modified(config) { @@ -490,11 +490,11 @@ fn run(self, builder: &Builder<'_>) -> LlvmResult { let mut cmd = Command::new(&res.llvm_config); let version = output(cmd.arg("--version")); let major = version.split('.').next().unwrap(); - let lib_name = match &llvm_version_suffix { + + match &llvm_version_suffix { Some(version_suffix) => format!("libLLVM-{major}{version_suffix}.{extension}"), None => format!("libLLVM-{major}.{extension}"), - }; - lib_name + } }; // When building LLVM with LLVM_LINK_LLVM_DYLIB for macOS, an unversioned @@ -749,13 +749,15 @@ fn configure_cmake( // For distribution we want the LLVM tools to be *statically* linked to libstdc++. // We also do this if the user explicitly requested static libstdc++. - if builder.config.llvm_static_stdcpp { - if !target.contains("msvc") && !target.contains("netbsd") && !target.contains("solaris") { - if target.contains("apple") || target.contains("windows") { - ldflags.push_all("-static-libstdc++"); - } else { - ldflags.push_all("-Wl,-Bsymbolic -static-libstdc++"); - } + if builder.config.llvm_static_stdcpp + && !target.contains("msvc") + && !target.contains("netbsd") + && !target.contains("solaris") + { + if target.contains("apple") || target.contains("windows") { + ldflags.push_all("-static-libstdc++"); + } else { + ldflags.push_all("-Wl,-Bsymbolic -static-libstdc++"); } } diff --git a/src/bootstrap/sanity.rs b/src/bootstrap/sanity.rs index 7e83b508e94..0e1c855ea3e 100644 --- a/src/bootstrap/sanity.rs +++ b/src/bootstrap/sanity.rs @@ -92,20 +92,19 @@ pub fn check(build: &mut Build) { .unwrap_or(true) }) .any(|build_llvm_ourselves| build_llvm_ourselves); + let need_cmake = building_llvm || build.config.any_sanitizers_enabled(); - if need_cmake { - if cmd_finder.maybe_have("cmake").is_none() { - eprintln!( - " + if need_cmake && cmd_finder.maybe_have("cmake").is_none() { + eprintln!( + " Couldn't find required command: cmake You should install cmake, or set `download-ci-llvm = true` in the `[llvm]` section of `config.toml` to download LLVM rather than building it. " - ); - crate::exit!(1); - } + ); + crate::exit!(1); } build.config.python = build @@ -199,10 +198,10 @@ pub fn check(build: &mut Build) { .entry(*target) .or_insert_with(|| Target::from_triple(&target.triple)); - if target.contains("-none-") || target.contains("nvptx") { - if build.no_std(*target) == Some(false) { - panic!("All the *-none-* and nvptx* targets are no-std targets") - } + if (target.contains("-none-") || target.contains("nvptx")) + && build.no_std(*target) == Some(false) + { + panic!("All the *-none-* and nvptx* targets are no-std targets") } // Make sure musl-root is valid diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index d1018978f78..e3a0580b2cc 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1139,16 +1139,14 @@ fn run(self, builder: &Builder<'_>) { .map(|filename| builder.src.join("src/etc/completions").join(filename)); if builder.config.cmd.bless() { builder.ensure(crate::run::GenerateCompletions); - } else { - if crate::flags::get_completion(shells::Bash, &bash).is_some() - || crate::flags::get_completion(shells::Fish, &fish).is_some() - || crate::flags::get_completion(shells::PowerShell, &powershell).is_some() - { - eprintln!( - "x.py completions were changed; run `x.py run generate-completions` to update them" - ); - crate::exit!(1); - } + } else if crate::flags::get_completion(shells::Bash, &bash).is_some() + || crate::flags::get_completion(shells::Fish, &fish).is_some() + || crate::flags::get_completion(shells::PowerShell, &powershell).is_some() + { + eprintln!( + "x.py completions were changed; run `x.py run generate-completions` to update them" + ); + crate::exit!(1); } } @@ -1372,7 +1370,7 @@ fn run(self, builder: &Builder<'_>) { let run = |target| { builder.ensure(Compiletest { compiler: self.compiler, - target: target, + target, mode: "mir-opt", suite: "mir-opt", path: "tests/mir-opt",