Skip to content

Commit

Permalink
Change flag ordering (#1403)
Browse files Browse the repository at this point in the history
  • Loading branch information
@madsmtm
madsmtm authored Feb 24, 2025
1 parent 5fbe4d5 commit 181c03d
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 45 deletions.
78 changes: 44 additions & 34 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1892,58 +1892,80 @@ impl Build {

let mut cmd = self.get_base_compiler()?;

// The flags below are added in roughly the following order:
// 1. Default flags
// - Controlled by `cc-rs`.
// 2. `rustc`-inherited flags
// - Controlled by `rustc`.
// 3. Builder flags
// - Controlled by the developer using `cc-rs` in e.g. their `build.rs`.
// 4. Environment flags
// - Controlled by the end user.
//
// This is important to allow later flags to override previous ones.

// Copied from <https://github.com/rust-lang/rust/blob/5db81020006d2920fc9c62ffc0f4322f90bffa04/compiler/rustc_codegen_ssa/src/back/linker.rs#L27-L38>
//
// Disables non-English messages from localized linkers.
// Such messages may cause issues with text encoding on Windows
// and prevent inspection of msvc output in case of errors, which we occasionally do.
// This should be acceptable because other messages from rustc are in English anyway,
// and may also be desirable to improve searchability of the compiler diagnostics.
if matches!(cmd.family, ToolFamily::Msvc { clang_cl: false }) {
cmd.env.push(("VSLANG".into(), "1033".into()));
} else {
cmd.env.push(("LC_ALL".into(), "C".into()));
}

// Disable default flag generation via `no_default_flags` or environment variable
let no_defaults = self.no_default_flags || self.getenv_boolean("CRATE_CC_NO_DEFAULTS");

if !no_defaults {
self.add_default_flags(&mut cmd, &target, &opt_level)?;
}

// Specify various flags that are not considered part of the default flags above.
// FIXME(madsmtm): Should these be considered part of the defaults? If no, why not?
if let Some(ref std) = self.std {
let separator = match cmd.family {
ToolFamily::Msvc { .. } => ':',
ToolFamily::Gnu | ToolFamily::Clang { .. } => '=',
};
cmd.push_cc_arg(format!("-std{}{}", separator, std).into());
}

for directory in self.include_directories.iter() {
cmd.args.push("-I".into());
cmd.args.push(directory.as_os_str().into());
}

let flags = self.envflags(if self.cpp { "CXXFLAGS" } else { "CFLAGS" })?;
if let Some(flags) = &flags {
for arg in flags {
cmd.push_cc_arg(arg.into());
}
if self.warnings_into_errors {
let warnings_to_errors_flag = cmd.family.warnings_to_errors_flag().into();
cmd.push_cc_arg(warnings_to_errors_flag);
}

// If warnings and/or extra_warnings haven't been explicitly set,
// then we set them only if the environment doesn't already have
// CFLAGS/CXXFLAGS, since those variables presumably already contain
// the desired set of warnings flags.

if self.warnings.unwrap_or(flags.is_none()) {
let envflags = self.envflags(if self.cpp { "CXXFLAGS" } else { "CFLAGS" })?;
if self.warnings.unwrap_or(envflags.is_none()) {
let wflags = cmd.family.warnings_flags().into();
cmd.push_cc_arg(wflags);
}

if self.extra_warnings.unwrap_or(flags.is_none()) {
if self.extra_warnings.unwrap_or(envflags.is_none()) {
if let Some(wflags) = cmd.family.extra_warnings_flags() {
cmd.push_cc_arg(wflags.into());
}
}

for flag in self.flags.iter() {
cmd.args.push((**flag).into());
}

// Add cc flags inherited from matching rustc flags
// Add cc flags inherited from matching rustc flags.
if self.inherit_rustflags {
self.add_inherited_rustflags(&mut cmd, &target)?;
}

// Set flags configured in the builder (do this second-to-last, to allow these to override
// everything above).
for flag in self.flags.iter() {
cmd.args.push((**flag).into());
}
for flag in self.flags_supported.iter() {
if self
.is_flag_supported_inner(flag, &cmd, &target)
Expand All @@ -1952,7 +1974,6 @@ impl Build {
cmd.push_cc_arg((**flag).into());
}
}

for (key, value) in self.definitions.iter() {
if let Some(ref value) = *value {
cmd.args.push(format!("-D{}={}", key, value).into());
Expand All @@ -1961,22 +1982,11 @@ impl Build {
}
}

if self.warnings_into_errors {
let warnings_to_errors_flag = cmd.family.warnings_to_errors_flag().into();
cmd.push_cc_arg(warnings_to_errors_flag);
}

// Copied from <https://github.com/rust-lang/rust/blob/5db81020006d2920fc9c62ffc0f4322f90bffa04/compiler/rustc_codegen_ssa/src/back/linker.rs#L27-L38>
//
// Disables non-English messages from localized linkers.
// Such messages may cause issues with text encoding on Windows
// and prevent inspection of msvc output in case of errors, which we occasionally do.
// This should be acceptable because other messages from rustc are in English anyway,
// and may also be desirable to improve searchability of the compiler diagnostics.
if matches!(cmd.family, ToolFamily::Msvc { clang_cl: false }) {
cmd.env.push(("VSLANG".into(), "1033".into()));
} else {
cmd.env.push(("LC_ALL".into(), "C".into()));
// Set flags from the environment (do this last, to allow these to override everything else).
if let Some(flags) = &envflags {
for arg in flags {
cmd.push_cc_arg(arg.into());
}
}

Ok(cmd)
Expand Down
36 changes: 27 additions & 9 deletions tests/cflags.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,39 @@ fn gnu_no_warnings_if_cflags() {
test.cmd(0).must_not_have("-Wall").must_not_have("-Wextra");
}

/// Test the ordering of `CFLAGS*` variables.
/// Test the ordering of flags.
///
/// 1. Default flags
/// 2. Rustflags.
/// 3. Builder flags.
/// 4. Environment flags.
fn cflags_order() {
unsafe { env::set_var("CFLAGS", "-arbitrary1") };
unsafe { env::set_var("HOST_CFLAGS", "-arbitrary2") };
unsafe { env::set_var("TARGET_CFLAGS", "-arbitrary2") };
unsafe { env::set_var("CFLAGS_x86_64_unknown_none", "-arbitrary3") };
unsafe { env::set_var("CFLAGS_x86_64-unknown-none", "-arbitrary4") };
// FIXME(madsmtm): Re-enable once `is_flag_supported` works in CI regardless of `target`.
// unsafe { std::env::set_var("CARGO_ENCODED_RUSTFLAGS", "-Cdwarf-version=5") };

unsafe { env::set_var("CFLAGS", "-Larbitrary1") };
unsafe { env::set_var("HOST_CFLAGS", "-Larbitrary2") };
unsafe { env::set_var("TARGET_CFLAGS", "-Larbitrary2") };
unsafe { env::set_var("CFLAGS_x86_64_unknown_none", "-Larbitrary3") };
unsafe { env::set_var("CFLAGS_x86_64-unknown-none", "-Larbitrary4") };

let test = Test::gnu();
test.gcc()
.target("x86_64-unknown-none")
.static_flag(true)
.flag("-Lbuilder-flag1")
.flag("-Lbuilder-flag2")
.file("foo.c")
.compile("foo");

test.cmd(0)
.must_have_in_order("-arbitrary1", "-arbitrary2")
.must_have_in_order("-arbitrary2", "-arbitrary3")
.must_have_in_order("-arbitrary3", "-arbitrary4");
// .must_have_in_order("-static", "-gdwarf-5")
// .must_have_in_order("-gdwarf-5", "-Lbuilder-flag1")
.must_have_in_order("-static", "-Lbuilder-flag1")
.must_have_in_order("-Lbuilder-flag1", "-Lbuilder-flag2")
.must_have_in_order("-Lbuilder-flag2", "-Larbitrary1")
.must_have_in_order("-Larbitrary1", "-Larbitrary2")
.must_have_in_order("-Larbitrary1", "-Larbitrary2")
.must_have_in_order("-Larbitrary2", "-Larbitrary3")
.must_have_in_order("-Larbitrary3", "-Larbitrary4");
}
4 changes: 2 additions & 2 deletions tests/support/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ impl Execution {
match (before_position, after_position) {
(Some(b), Some(a)) if b < a => {}
(b, a) => panic!(
"{:?} (last position: {:?}) did not appear before {:?} (last position: {:?})",
before, b, after, a
"{:?} (last position: {:?}) did not appear before {:?} (last position: {:?}): {:?}",
before, b, after, a, self.args
),
};
self
Expand Down

0 comments on commit 181c03d

Please sign in to comment.