From 85401882d23f8edc946ef5e08e8c254da4ac1936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 12 Jun 2025 15:18:21 +0200 Subject: [PATCH] Remove `RefCell` from `cc/cxx/ar/ranlib` It wasn't really needed there. --- src/bootstrap/src/core/build_steps/test.rs | 5 ++- src/bootstrap/src/core/build_steps/tool.rs | 2 +- src/bootstrap/src/core/builder/cargo.rs | 4 +-- src/bootstrap/src/lib.rs | 38 +++++++++++----------- src/bootstrap/src/utils/cc_detect.rs | 16 ++++----- src/bootstrap/src/utils/cc_detect/tests.rs | 26 +++++++-------- src/bootstrap/src/utils/tests/mod.rs | 2 ++ 7 files changed, 44 insertions(+), 49 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index ebb926d81cef..8e21da0050f3 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2009,7 +2009,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the // Note that if we encounter `PATH` we make sure to append to our own `PATH` // rather than stomp over it. if !builder.config.dry_run() && target.is_msvc() { - for (k, v) in builder.cc.borrow()[&target].env() { + for (k, v) in builder.cc[&target].env() { if k != "PATH" { cmd.env(k, v); } @@ -2026,8 +2026,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the // address sanitizer enabled (e.g., ntdll.dll). cmd.env("ASAN_WIN_CONTINUE_ON_INTERCEPTION_FAILURE", "1"); // Add the address sanitizer runtime to the PATH - it is located next to cl.exe. - let asan_runtime_path = - builder.cc.borrow()[&target].path().parent().unwrap().to_path_buf(); + let asan_runtime_path = builder.cc[&target].path().parent().unwrap().to_path_buf(); let old_path = cmd .get_envs() .find_map(|(k, v)| (k == "PATH").then_some(v)) diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 680718f05529..3c7c42f43dba 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -1321,7 +1321,7 @@ impl Builder<'_> { if compiler.host.is_msvc() { let curpaths = env::var_os("PATH").unwrap_or_default(); let curpaths = env::split_paths(&curpaths).collect::>(); - for (k, v) in self.cc.borrow()[&compiler.host].env() { + for (k, v) in self.cc[&compiler.host].env() { if k != "PATH" { continue; } diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index c6cfdb69356c..0e3c3aaee0ff 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/src/bootstrap/src/core/builder/cargo.rs @@ -278,9 +278,7 @@ impl Cargo { self.rustdocflags.arg(&arg); } - if !builder.config.dry_run() - && builder.cc.borrow()[&target].args().iter().any(|arg| arg == "-gz") - { + if !builder.config.dry_run() && builder.cc[&target].args().iter().any(|arg| arg == "-gz") { self.rustflags.arg("-Clink-arg=-gz"); } diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 25e59bfe3a88..5303e9a91b6f 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -17,7 +17,7 @@ //! also check out the `src/bootstrap/README.md` file for more information. #![cfg_attr(test, allow(unused))] -use std::cell::{Cell, RefCell}; +use std::cell::Cell; use std::collections::{BTreeSet, HashMap, HashSet}; use std::fmt::Display; use std::path::{Path, PathBuf}; @@ -190,10 +190,10 @@ pub struct Build { // Runtime state filled in later on // C/C++ compilers and archiver for all targets - cc: RefCell>, - cxx: RefCell>, - ar: RefCell>, - ranlib: RefCell>, + cc: HashMap, + cxx: HashMap, + ar: HashMap, + ranlib: HashMap, // Miscellaneous // allow bidirectional lookups: both name -> path and path -> name crates: HashMap, @@ -464,10 +464,10 @@ impl Build { enzyme_info, in_tree_llvm_info, in_tree_gcc_info, - cc: RefCell::new(HashMap::new()), - cxx: RefCell::new(HashMap::new()), - ar: RefCell::new(HashMap::new()), - ranlib: RefCell::new(HashMap::new()), + cc: HashMap::new(), + cxx: HashMap::new(), + ar: HashMap::new(), + ranlib: HashMap::new(), crates: HashMap::new(), crate_paths: HashMap::new(), is_sudo, @@ -493,7 +493,7 @@ impl Build { } build.verbose(|| println!("finding compilers")); - utils::cc_detect::find(&build); + utils::cc_detect::fill_compilers(&mut build); // When running `setup`, the profile is about to change, so any requirements we have now may // be different on the next invocation. Don't check for them until the next time x.py is // run. This is ok because `setup` never runs any build commands, so it won't fail if commands are missing. @@ -1133,17 +1133,17 @@ impl Build { if self.config.dry_run() { return PathBuf::new(); } - self.cc.borrow()[&target].path().into() + self.cc[&target].path().into() } /// Returns the internal `cc::Tool` for the C compiler. fn cc_tool(&self, target: TargetSelection) -> Tool { - self.cc.borrow()[&target].clone() + self.cc[&target].clone() } /// Returns the internal `cc::Tool` for the C++ compiler. fn cxx_tool(&self, target: TargetSelection) -> Tool { - self.cxx.borrow()[&target].clone() + self.cxx[&target].clone() } /// Returns C flags that `cc-rs` thinks should be enabled for the @@ -1153,8 +1153,8 @@ impl Build { return Vec::new(); } let base = match c { - CLang::C => self.cc.borrow()[&target].clone(), - CLang::Cxx => self.cxx.borrow()[&target].clone(), + CLang::C => self.cc[&target].clone(), + CLang::Cxx => self.cxx[&target].clone(), }; // Filter out -O and /O (the optimization flags) that we picked up @@ -1207,7 +1207,7 @@ impl Build { if self.config.dry_run() { return None; } - self.ar.borrow().get(&target).cloned() + self.ar.get(&target).cloned() } /// Returns the path to the `ranlib` utility for the target specified. @@ -1215,7 +1215,7 @@ impl Build { if self.config.dry_run() { return None; } - self.ranlib.borrow().get(&target).cloned() + self.ranlib.get(&target).cloned() } /// Returns the path to the C++ compiler for the target specified. @@ -1223,7 +1223,7 @@ impl Build { if self.config.dry_run() { return Ok(PathBuf::new()); } - match self.cxx.borrow().get(&target) { + match self.cxx.get(&target) { Some(p) => Ok(p.path().into()), None => Err(format!("target `{target}` is not configured as a host, only as a target")), } @@ -1240,7 +1240,7 @@ impl Build { } else if target.contains("vxworks") { // need to use CXX compiler as linker to resolve the exception functions // that are only existed in CXX libraries - Some(self.cxx.borrow()[&target].path().into()) + Some(self.cxx[&target].path().into()) } else if !self.config.is_host_target(target) && helpers::use_host_linker(target) && !target.is_msvc() diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 851bb38074ef..2af30c16f8fd 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -61,8 +61,8 @@ fn new_cc_build(build: &Build, target: TargetSelection) -> cc::Build { /// /// This function determines which targets need a C compiler (and, if needed, a C++ compiler) /// by combining the primary build target, host targets, and any additional targets. For -/// each target, it calls [`find_target`] to configure the necessary compiler tools. -pub fn find(build: &Build) { +/// each target, it calls [`fill_target_compiler`] to configure the necessary compiler tools. +pub fn fill_compilers(build: &mut Build) { let targets: HashSet<_> = match build.config.cmd { // We don't need to check cross targets for these commands. crate::Subcommand::Clean { .. } @@ -87,7 +87,7 @@ pub fn find(build: &Build) { }; for target in targets.into_iter() { - find_target(build, target); + fill_target_compiler(build, target); } } @@ -96,7 +96,7 @@ pub fn find(build: &Build) { /// This function uses both user-specified configuration (from `bootstrap.toml`) and auto-detection /// logic to determine the correct C/C++ compilers for the target. It also determines the appropriate /// archiver (`ar`) and sets up additional compilation flags (both handled and unhandled). -pub fn find_target(build: &Build, target: TargetSelection) { +pub fn fill_target_compiler(build: &mut Build, target: TargetSelection) { let mut cfg = new_cc_build(build, target); let config = build.config.target_config.get(&target); if let Some(cc) = config @@ -113,7 +113,7 @@ pub fn find_target(build: &Build, target: TargetSelection) { cfg.try_get_archiver().map(|c| PathBuf::from(c.get_program())).ok() }; - build.cc.borrow_mut().insert(target, compiler.clone()); + build.cc.insert(target, compiler.clone()); let mut cflags = build.cc_handled_clags(target, CLang::C); cflags.extend(build.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::C)); @@ -135,7 +135,7 @@ pub fn find_target(build: &Build, target: TargetSelection) { // for VxWorks, record CXX compiler which will be used in lib.rs:linker() if cxx_configured || target.contains("vxworks") { let compiler = cfg.get_compiler(); - build.cxx.borrow_mut().insert(target, compiler); + build.cxx.insert(target, compiler); } build.verbose(|| println!("CC_{} = {:?}", target.triple, build.cc(target))); @@ -148,11 +148,11 @@ pub fn find_target(build: &Build, target: TargetSelection) { } if let Some(ar) = ar { build.verbose(|| println!("AR_{} = {ar:?}", target.triple)); - build.ar.borrow_mut().insert(target, ar); + build.ar.insert(target, ar); } if let Some(ranlib) = config.and_then(|c| c.ranlib.clone()) { - build.ranlib.borrow_mut().insert(target, ranlib); + build.ranlib.insert(target, ranlib); } } diff --git a/src/bootstrap/src/utils/cc_detect/tests.rs b/src/bootstrap/src/utils/cc_detect/tests.rs index 1f50738959fb..9ddf91fcc858 100644 --- a/src/bootstrap/src/utils/cc_detect/tests.rs +++ b/src/bootstrap/src/utils/cc_detect/tests.rs @@ -119,18 +119,14 @@ fn test_find_target_with_config() { target_config.ar = Some(PathBuf::from("dummy-ar")); target_config.ranlib = Some(PathBuf::from("dummy-ranlib")); build.config.target_config.insert(target.clone(), target_config); - find_target(&build, target.clone()); - let binding = build.cc.borrow(); - let cc_tool = binding.get(&target).unwrap(); + fill_target_compiler(&mut build, target.clone()); + let cc_tool = build.cc.get(&target).unwrap(); assert_eq!(cc_tool.path(), &PathBuf::from("dummy-cc")); - let binding = build.cxx.borrow(); - let cxx_tool = binding.get(&target).unwrap(); + let cxx_tool = build.cxx.get(&target).unwrap(); assert_eq!(cxx_tool.path(), &PathBuf::from("dummy-cxx")); - let binding = build.ar.borrow(); - let ar = binding.get(&target).unwrap(); + let ar = build.ar.get(&target).unwrap(); assert_eq!(ar, &PathBuf::from("dummy-ar")); - let binding = build.ranlib.borrow(); - let ranlib = binding.get(&target).unwrap(); + let ranlib = build.ranlib.get(&target).unwrap(); assert_eq!(ranlib, &PathBuf::from("dummy-ranlib")); } @@ -139,12 +135,12 @@ fn test_find_target_without_config() { let mut build = Build::new(Config { ..Config::parse(Flags::parse(&["build".to_owned()])) }); let target = TargetSelection::from_user("x86_64-unknown-linux-gnu"); build.config.target_config.clear(); - find_target(&build, target.clone()); - assert!(build.cc.borrow().contains_key(&target)); + fill_target_compiler(&mut build, target.clone()); + assert!(build.cc.contains_key(&target)); if !target.triple.contains("vxworks") { - assert!(build.cxx.borrow().contains_key(&target)); + assert!(build.cxx.contains_key(&target)); } - assert!(build.ar.borrow().contains_key(&target)); + assert!(build.ar.contains_key(&target)); } #[test] @@ -154,8 +150,8 @@ fn test_find() { let target2 = TargetSelection::from_user("x86_64-unknown-openbsd"); build.targets.push(target1.clone()); build.hosts.push(target2.clone()); - find(&build); + fill_compilers(&mut build); for t in build.hosts.iter().chain(build.targets.iter()).chain(iter::once(&build.host_target)) { - assert!(build.cc.borrow().contains_key(t), "CC not set for target {}", t.triple); + assert!(build.cc.contains_key(t), "CC not set for target {}", t.triple); } } diff --git a/src/bootstrap/src/utils/tests/mod.rs b/src/bootstrap/src/utils/tests/mod.rs index c2bf1c3ee8a8..73c500f6e369 100644 --- a/src/bootstrap/src/utils/tests/mod.rs +++ b/src/bootstrap/src/utils/tests/mod.rs @@ -1 +1,3 @@ +//! This module contains shared utilities for bootstrap tests. + pub mod git;