From e790b92d72246c5dc2752285d1bb5605af3d2f64 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 31 Jan 2025 04:07:04 +0100 Subject: Bootstrap: Don't duplicate cc-rs flags Commands that end up invoking cc-rs, i.e. Cargo (through build scripts) and cmake-rs don't need the CFLAGS from cc-rs itself, as they will just end up as duplicates. We do still choose to pass them in certain places, but now it's at least clear which flags are default, and which flags are extra flags added on. --- src/bootstrap/src/core/build_steps/compile.rs | 3 ++- src/bootstrap/src/core/build_steps/llvm.rs | 11 +++++++++-- src/bootstrap/src/core/build_steps/test.rs | 8 ++++++-- src/bootstrap/src/core/builder/cargo.rs | 4 ++-- src/bootstrap/src/lib.rs | 21 +++++++++++++-------- src/bootstrap/src/utils/cc_detect.rs | 6 ++++-- 6 files changed, 36 insertions(+), 17 deletions(-) (limited to 'src/bootstrap') diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index fd9bf47234c..d76a15fa583 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1541,7 +1541,8 @@ pub fn compiler_file( return PathBuf::new(); } let mut cmd = command(compiler); - cmd.args(builder.cflags(target, GitRepo::Rustc, c)); + cmd.args(builder.default_cflags(target, c)); + cmd.args(builder.extra_cflags(target, GitRepo::Rustc, c)); cmd.arg(format!("-print-file-name={file}")); let out = cmd.run_capture_stdout(builder).stdout(); PathBuf::from(out.trim()) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 3cf25373b89..706bef521e0 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -757,9 +757,15 @@ fn configure_cmake( } cfg.build_arg("-j").build_arg(builder.jobs().to_string()); + // FIXME(madsmtm): Allow `cmake-rs` to select flags by itself by passing + // our flags via `.cflag`/`.cxxflag` instead. + // + // Needs `suppressed_compiler_flag_prefixes` to be gone, and hence + // https://github.com/llvm/llvm-project/issues/88780 to be fixed. let mut cflags: OsString = builder - .cflags(target, GitRepo::Llvm, CLang::C) + .default_cflags(target, CLang::C) .into_iter() + .chain(builder.extra_cflags(target, GitRepo::Llvm, CLang::C)) .filter(|flag| { !suppressed_compiler_flag_prefixes .iter() @@ -778,8 +784,9 @@ fn configure_cmake( } cfg.define("CMAKE_C_FLAGS", cflags); let mut cxxflags: OsString = builder - .cflags(target, GitRepo::Llvm, CLang::Cxx) + .default_cflags(target, CLang::Cxx) .into_iter() + .chain(builder.extra_cflags(target, GitRepo::Llvm, CLang::Cxx)) .filter(|flag| { !suppressed_compiler_flag_prefixes .iter() diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 99b033db115..a5cad5b8414 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2005,14 +2005,18 @@ 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" { + let mut cflags = builder.default_cflags(target, CLang::C); + cflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::C)); + let mut cxxflags = builder.default_cflags(target, CLang::Cxx); + cxxflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx)); cmd.arg("--cc") .arg(builder.cc(target)) .arg("--cxx") .arg(builder.cxx(target).unwrap()) .arg("--cflags") - .arg(builder.cflags(target, GitRepo::Rustc, CLang::C).join(" ")) + .arg(cflags.join(" ")) .arg("--cxxflags") - .arg(builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" ")); + .arg(cxxflags.join(" ")); copts_passed = true; if let Some(ar) = builder.ar(target) { cmd.arg("--ar").arg(ar); diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index d418237a568..fb8ef2b57be 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/src/bootstrap/src/core/builder/cargo.rs @@ -317,7 +317,7 @@ impl Cargo { let cc = ccacheify(&builder.cc(target)); self.command.env(format!("CC_{triple_underscored}"), &cc); - let cflags = builder.cflags(target, GitRepo::Rustc, CLang::C).join(" "); + let cflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::C).join(" "); self.command.env(format!("CFLAGS_{triple_underscored}"), &cflags); if let Some(ar) = builder.ar(target) { @@ -329,7 +329,7 @@ impl Cargo { if let Ok(cxx) = builder.cxx(target) { let cxx = ccacheify(&cxx); - let cxxflags = builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "); + let cxxflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "); self.command .env(format!("CXX_{triple_underscored}"), &cxx) .env(format!("CXXFLAGS_{triple_underscored}"), cxxflags); diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 482e23cd04c..1ce86831662 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -246,6 +246,7 @@ impl Mode { } } +#[derive(Debug, Hash, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub enum CLang { C, Cxx, @@ -1139,9 +1140,9 @@ Executed at: {executed_at}"#, self.cc.borrow()[&target].path().into() } - /// Returns a list of flags to pass to the C compiler for the target - /// specified. - fn cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec { + /// Returns C flags that `cc-rs` thinks should be enabled for the + /// specified target by default. + fn default_cflags(&self, target: TargetSelection, c: CLang) -> Vec { if self.config.dry_run() { return Vec::new(); } @@ -1150,14 +1151,18 @@ Executed at: {executed_at}"#, CLang::Cxx => self.cxx.borrow()[&target].clone(), }; - // Filter out -O and /O (the optimization flags) that we picked up from - // cc-rs because the build scripts will determine that for themselves. - let mut base = base - .args() + // Filter out -O and /O (the optimization flags) that we picked up + // from cc-rs, that's up to the caller to figure out. + base.args() .iter() .map(|s| s.to_string_lossy().into_owned()) .filter(|s| !s.starts_with("-O") && !s.starts_with("/O")) - .collect::>(); + .collect::>() + } + + /// Returns extra C flags that `cc-rs` doesn't handle. + fn extra_cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec { + let mut base = Vec::new(); // If we're compiling C++ on macOS then we add a flag indicating that // we want libc++ (more filled out than libstdc++), ensuring that diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 4aec554b432..33b490e5ca1 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -142,7 +142,8 @@ pub fn find_target(build: &Build, target: TargetSelection) { }; build.cc.borrow_mut().insert(target, compiler.clone()); - let cflags = build.cflags(target, GitRepo::Rustc, CLang::C); + let mut cflags = build.default_cflags(target, CLang::C); + cflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::C)); // If we use llvm-libunwind, we will need a C++ compiler as well for all targets // We'll need one anyways if the target triple is also a host triple @@ -168,7 +169,8 @@ pub fn find_target(build: &Build, target: TargetSelection) { build.verbose(|| println!("CC_{} = {:?}", target.triple, build.cc(target))); build.verbose(|| println!("CFLAGS_{} = {cflags:?}", target.triple)); if let Ok(cxx) = build.cxx(target) { - let cxxflags = build.cflags(target, GitRepo::Rustc, CLang::Cxx); + let mut cxxflags = build.default_cflags(target, CLang::Cxx); + cxxflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::Cxx)); build.verbose(|| println!("CXX_{} = {cxx:?}", target.triple)); build.verbose(|| println!("CXXFLAGS_{} = {cxxflags:?}", target.triple)); } -- cgit 1.4.1-3-g733a5