Skip to content

Commit

Permalink
tool: Expose family() in favour of incomplete is_like_xxx() wrappers
Browse files Browse the repository at this point in the history 
Instead of adding a new `is_like_xxx()` function every time new
information is needed, or new enumeration variants would be added,
provide the (now `non_exhaustive`) `enum` directly to callers to match
what they're interested in.

Also removes `is_like_clang_cl()` again from rust-lang#1357 since that did not
yet make it into a release, and would only cause unnecessary duplication
in our API patterns.
  • Loading branch information
@MarijnS95
MarijnS95 committed Jan 10, 2025
1 parent 29a92bd commit cc342c5
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 38 deletions.
17 changes: 10 additions & 7 deletions dev-tools/cc-test/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ fn main() {
run_forked_capture_output(&out, "metadata-off");

run_forked_capture_output(&out, "warnings-off");
if cc::Build::new().get_compiler().is_like_msvc() {
if matches!(
cc::Build::new().get_compiler().family(),
cc::ToolFamily::Msvc { .. }
) {
// MSVC doesn't output warnings to stderr, so we can't capture them.
// the test will use this env var to know whether to run the test.
println!("cargo:rustc-env=TEST_WARNINGS_ON=0");
Expand Down Expand Up @@ -86,12 +89,12 @@ fn main() {
}

if target.contains("msvc") {
let cc_frontend = if compiler.is_like_msvc() {
"MSVC"
} else if compiler.is_like_clang() {
"CLANG"
} else {
unimplemented!("Unknown compiler that targets msvc but isn't clang-like or msvc-like")
let cc_frontend = match compiler.family() {
cc::ToolFamily::Clang { .. } => "CLANG",
cc::ToolFamily::Msvc { .. } => "MSVC",
f => unimplemented!(
"Unknown compiler `{f:?}` that targets msvc but isn't clang-like or msvc-like"
),
};

// Test that the `windows_registry` module will set PATH by looking for
Expand Down
38 changes: 20 additions & 18 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,7 @@ mod command_helpers;
use command_helpers::*;

mod tool;
pub use tool::Tool;
use tool::ToolFamily;
pub use tool::{Tool, ToolFamily};

mod tempfile;

Expand Down Expand Up @@ -701,25 +700,25 @@ impl Build {
if compiler.family.verbose_stderr() {
compiler.remove_arg("-v".into());
}
if compiler.is_like_clang() {
let clang = matches!(compiler.family(), ToolFamily::Clang { .. });
if clang {
// Avoid reporting that the arg is unsupported just because the
// compiler complains that it wasn't used.
compiler.push_cc_arg("-Wno-unused-command-line-argument".into());
}

let mut cmd = compiler.to_command();
let is_arm = matches!(target.arch, "aarch64" | "arm");
let clang = compiler.is_like_clang();
let gnu = compiler.family == ToolFamily::Gnu;
let msvc = matches!(compiler.family(), ToolFamily::Msvc { .. });
command_add_output_file(
&mut cmd,
&obj,
CmdAddOutputFileArgs {
cuda: self.cuda,
is_assembler_msvc: false,
msvc: compiler.is_like_msvc(),
msvc,
clang,
gnu,
gnu: matches!(compiler.family(), ToolFamily::Gnu),
is_asm: false,
is_arm,
},
Expand All @@ -733,7 +732,7 @@ impl Build {

// On MSVC skip the CRT by setting the entry point to `main`.
// This way we don't need to add the default library paths.
if compiler.is_like_msvc() {
if msvc {
// Flags from _LINK_ are appended to the linker arguments.
cmd.env("_LINK_", "-entry:main");
}
Expand Down Expand Up @@ -1753,8 +1752,6 @@ impl Build {
let target = self.get_target()?;
let msvc = target.env == "msvc";
let compiler = self.try_get_compiler()?;
let clang = compiler.is_like_clang();
let gnu = compiler.family == ToolFamily::Gnu;

let is_assembler_msvc = msvc && asm_ext == Some(AsmFileExt::DotAsm);
let (mut cmd, name) = if is_assembler_msvc {
Expand Down Expand Up @@ -1782,9 +1779,9 @@ impl Build {
CmdAddOutputFileArgs {
cuda: self.cuda,
is_assembler_msvc,
msvc: compiler.is_like_msvc(),
clang,
gnu,
msvc: matches!(compiler.family(), ToolFamily::Msvc { .. }),
clang: matches!(compiler.family(), ToolFamily::Clang { .. }),
gnu: matches!(compiler.family(), ToolFamily::Gnu),
is_asm,
is_arm,
},
Expand Down Expand Up @@ -2036,15 +2033,16 @@ impl Build {
}
}
ToolFamily::Gnu | ToolFamily::Clang { .. } => {
let clang = matches!(cmd.family, ToolFamily::Clang { .. });
// arm-linux-androideabi-gcc 4.8 shipped with Android NDK does
// not support '-Oz'
if opt_level == "z" && !cmd.is_like_clang() {
if opt_level == "z" && !clang {
cmd.push_opt_unless_duplicate("-Os".into());
} else {
cmd.push_opt_unless_duplicate(format!("-O{}", opt_level).into());
}

if cmd.is_like_clang() && target.os == "android" {
if clang && target.os == "android" {
// For compatibility with code that doesn't use pre-defined `__ANDROID__` macro.
// If compiler used via ndk-build or cmake (officially supported build methods)
// this macros is defined.
Expand Down Expand Up @@ -2141,7 +2139,9 @@ impl Build {
family.add_force_frame_pointer(cmd);
}

if !cmd.is_like_msvc() {
let msvc = matches!(cmd.family, ToolFamily::Msvc { .. });

if !msvc {
if target.arch == "x86" {
cmd.args.push("-m32".into());
} else if target.abi == "x32" {
Expand Down Expand Up @@ -2653,7 +2653,8 @@ impl Build {
// it does not support iOS in general), but we specify them anyhow in
// case we actually have a Clang-like compiler disguised as a GNU-like
// compiler, or in case GCC adds support for these in the future.
if !cmd.is_like_clang() {
let clang = matches!(cmd.family, ToolFamily::Clang { .. });
if !clang {
let min_version = self.apple_deployment_target(&target);
cmd.args
.push(target.apple_version_flag(&min_version).into());
Expand Down Expand Up @@ -3225,7 +3226,8 @@ impl Build {
// And even extend it to gcc targets by searching for "ar" instead
// of "llvm-ar"...
let compiler = self.get_base_compiler().ok()?;
if compiler.is_like_clang() {
let clang = matches!(compiler.family, ToolFamily::Clang { .. });
if clang {
name = format!("llvm-{}", tool).into();
self.search_programs(
&mut self.cmd(&compiler.path),
Expand Down
36 changes: 23 additions & 13 deletions src/tool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,11 +281,11 @@ impl Tool {
let mut chars = flag.chars();

// Only duplicate check compiler flags
if self.is_like_msvc() {
if chars.next() != Some('/') {
return false;
}
} else if (self.is_like_gnu() || self.is_like_clang()) && chars.next() != Some('-') {
let flag_start = match self.family {
ToolFamily::Msvc { .. } => '/',
ToolFamily::Gnu | ToolFamily::Clang { .. } => '-',
};
if chars.next() != Some(flag_start) {
return false;
}

Expand Down Expand Up @@ -395,6 +395,11 @@ impl Tool {
flags
}

/// The family of this tool, representing convention of arguments etc.
pub fn family(&self) -> ToolFamily {
self.family
}

/// Whether the tool is GNU Compiler Collection-like.
pub fn is_like_gnu(&self) -> bool {
self.family == ToolFamily::Gnu
Expand All @@ -421,11 +426,6 @@ impl Tool {
matches!(self.family, ToolFamily::Msvc { .. })
}

/// Whether the tool is `clang-cl`-based MSVC-like.
pub fn is_like_clang_cl(&self) -> bool {
matches!(self.family, ToolFamily::Msvc { clang_cl: true })
}

/// Supports using `--` delimiter to separate arguments and path to source files.
pub(crate) fn supports_path_delimiter(&self) -> bool {
matches!(
Expand All @@ -441,14 +441,24 @@ impl Tool {
///
/// Detection of a family is done on best-effort basis and may not accurately reflect the tool.
#[derive(Copy, Clone, Debug, PartialEq)]
#[non_exhaustive]
pub enum ToolFamily {
/// Tool is GNU Compiler Collection-like.
#[non_exhaustive]
Gnu,
/// Tool is Clang-like. It differs from the GCC in a sense that it accepts superset of flags
/// and its cross-compilation approach is different.
Clang { zig_cc: bool },
/// Tool is the MSVC cl.exe.
Msvc { clang_cl: bool },
#[non_exhaustive]
Clang {
/// Tool provided by zig
zig_cc: bool,
},
/// Tool is the MSVC `cl.exe`.
#[non_exhaustive]
Msvc {
/// Whether this is `clang-cl` provided by LLVM
clang_cl: bool,
},
}

impl ToolFamily {
Expand Down

0 comments on commit cc342c5

Please sign in to comment.