Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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.
  • Loading branch information
madsmtm committed Jan 31, 2025
commit e790b92d72246c5dc2752285d1bb5605af3d2f64
3 changes: 2 additions & 1 deletion src/bootstrap/src/core/build_steps/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
11 changes: 9 additions & 2 deletions src/bootstrap/src/core/build_steps/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down
8 changes: 6 additions & 2 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/src/core/builder/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
21 changes: 13 additions & 8 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ impl Mode {
}
}

#[derive(Debug, Hash, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub enum CLang {
C,
Cxx,
Expand Down Expand Up @@ -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<String> {
/// 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<String> {
if self.config.dry_run() {
return Vec::new();
}
Expand All @@ -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::<Vec<String>>();
.collect::<Vec<String>>()
}

/// Returns extra C flags that `cc-rs` doesn't handle.
fn extra_cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec<String> {
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
Expand Down
6 changes: 4 additions & 2 deletions src/bootstrap/src/utils/cc_detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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));
}
Expand Down