Skip to content

Commit

Permalink
Implement g++ and clang++ behavior when used on a .c input file.
Browse files Browse the repository at this point in the history
…Fixes mozilla#803

`g++` and `clang++` both compile `.c` files in C++ mode in the absence of any other directions (like the `-x` flag); add code to distinguish between the two compilers and their `++` variations, then fix up language selection and caching accordingly.
  • Loading branch information
Ricky Stewart committed Jul 27, 2020
1 parent 86c76c7 commit feca129
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 24 deletions.
75 changes: 59 additions & 16 deletions src/compiler/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ pub enum CCompilerKind {
pub trait CCompilerImpl: Clone + fmt::Debug + Send + 'static {
/// Return the kind of compiler.
fn kind(&self) -> CCompilerKind;
/// Return true iff this is g++ or clang++.
fn plusplus(&self) -> bool;
/// Determine whether `arguments` are supported by this compiler.
fn parse_arguments(
&self,
Expand Down Expand Up @@ -344,6 +346,7 @@ where
&extra_hashes,
&env_vars,
&preprocessor_result.stdout,
compiler.plusplus(),
)
};
// A compiler binary may be a symlink to another and so has the same digest, but that means
Expand Down Expand Up @@ -621,7 +624,7 @@ impl pkg::ToolchainPackager for CToolchainPackager {
}

/// The cache is versioned by the inputs to `hash_key`.
pub const CACHE_VERSION: &[u8] = b"9";
pub const CACHE_VERSION: &[u8] = b"10";

lazy_static! {
/// Environment variables that are factored into the cache key.
Expand All @@ -639,6 +642,7 @@ pub fn hash_key(
extra_hashes: &[String],
env_vars: &[(OsString, OsString)],
preprocessor_output: &[u8],
plusplus: bool,
) -> String {
// If you change any of the inputs to the hash, you should change `CACHE_VERSION`.
let mut m = Digest::new();
Expand All @@ -660,20 +664,43 @@ pub fn hash_key(
}
}
m.update(preprocessor_output);
// clang and clang++ have different behavior despite being byte-for-byte identical binaries, so
// we have to incorporate that into the hash as well.
m.update(&[plusplus as u8]);
m.finish()
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn test_same_content() {
let args = ovec!["a", "b", "c"];
const PREPROCESSED: &[u8] = b"hello world";
assert_eq!(
hash_key("abcd", Language::C, &args, &[], &[], &PREPROCESSED, false),
hash_key("abcd", Language::C, &args, &[], &[], &PREPROCESSED, false)
);
}

#[test]
fn test_plusplus_differs() {
let args = ovec!["a", "b", "c"];
const PREPROCESSED: &[u8] = b"hello world";
assert_neq!(
hash_key("abcd", Language::C, &args, &[], &[], &PREPROCESSED, false),
hash_key("abcd", Language::C, &args, &[], &[], &PREPROCESSED, true)
);
}

#[test]
fn test_hash_key_executable_contents_differs() {
let args = ovec!["a", "b", "c"];
const PREPROCESSED: &[u8] = b"hello world";
assert_neq!(
hash_key("abcd", Language::C, &args, &[], &[], &PREPROCESSED),
hash_key("wxyz", Language::C, &args, &[], &[], &PREPROCESSED)
hash_key("abcd", Language::C, &args, &[], &[], &PREPROCESSED, false),
hash_key("wxyz", Language::C, &args, &[], &[], &PREPROCESSED, false)
);
}

Expand All @@ -686,27 +713,35 @@ mod test {
let a = ovec!["a"];
const PREPROCESSED: &[u8] = b"hello world";
assert_neq!(
hash_key(digest, Language::C, &abc, &[], &[], &PREPROCESSED),
hash_key(digest, Language::C, &xyz, &[], &[], &PREPROCESSED)
hash_key(digest, Language::C, &abc, &[], &[], &PREPROCESSED, false),
hash_key(digest, Language::C, &xyz, &[], &[], &PREPROCESSED, false)
);

assert_neq!(
hash_key(digest, Language::C, &abc, &[], &[], &PREPROCESSED),
hash_key(digest, Language::C, &ab, &[], &[], &PREPROCESSED)
hash_key(digest, Language::C, &abc, &[], &[], &PREPROCESSED, false),
hash_key(digest, Language::C, &ab, &[], &[], &PREPROCESSED, false)
);

assert_neq!(
hash_key(digest, Language::C, &abc, &[], &[], &PREPROCESSED),
hash_key(digest, Language::C, &a, &[], &[], &PREPROCESSED)
hash_key(digest, Language::C, &abc, &[], &[], &PREPROCESSED, false),
hash_key(digest, Language::C, &a, &[], &[], &PREPROCESSED, false)
);
}

#[test]
fn test_hash_key_preprocessed_content_differs() {
let args = ovec!["a", "b", "c"];
assert_neq!(
hash_key("abcd", Language::C, &args, &[], &[], &b"hello world"[..]),
hash_key("abcd", Language::C, &args, &[], &[], &b"goodbye"[..])
hash_key(
"abcd",
Language::C,
&args,
&[],
&[],
&b"hello world"[..],
false
),
hash_key("abcd", Language::C, &args, &[], &[], &b"goodbye"[..], false)
);
}

Expand All @@ -716,11 +751,11 @@ mod test {
let digest = "abcd";
const PREPROCESSED: &[u8] = b"hello world";
for var in CACHED_ENV_VARS.iter() {
let h1 = hash_key(digest, Language::C, &args, &[], &[], &PREPROCESSED);
let h1 = hash_key(digest, Language::C, &args, &[], &[], &PREPROCESSED, false);
let vars = vec![(OsString::from(var), OsString::from("something"))];
let h2 = hash_key(digest, Language::C, &args, &[], &vars, &PREPROCESSED);
let h2 = hash_key(digest, Language::C, &args, &[], &vars, &PREPROCESSED, false);
let vars = vec![(OsString::from(var), OsString::from("something else"))];
let h3 = hash_key(digest, Language::C, &args, &[], &vars, &PREPROCESSED);
let h3 = hash_key(digest, Language::C, &args, &[], &vars, &PREPROCESSED, false);
assert_neq!(h1, h2);
assert_neq!(h2, h3);
}
Expand All @@ -734,8 +769,16 @@ mod test {
let extra_data = stringvec!["hello", "world"];

assert_neq!(
hash_key(digest, Language::C, &args, &extra_data, &[], &PREPROCESSED),
hash_key(digest, Language::C, &args, &[], &[], &PREPROCESSED)
hash_key(
digest,
Language::C,
&args,
&extra_data,
&[],
&PREPROCESSED,
false
),
hash_key(digest, Language::C, &args, &[], &[], &PREPROCESSED, false)
);
}
}
15 changes: 12 additions & 3 deletions src/compiler/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,20 @@ use std::process;

use crate::errors::*;

/// A unit struct on which to implement `CCompilerImpl`.
/// A struct on which to implement `CCompilerImpl`.
#[derive(Clone, Debug)]
pub struct Clang;
pub struct Clang {
/// true iff this is clang++.
pub clangplusplus: bool,
}

impl CCompilerImpl for Clang {
fn kind(&self) -> CCompilerKind {
CCompilerKind::Clang
}
fn plusplus(&self) -> bool {
self.clangplusplus
}
fn parse_arguments(
&self,
arguments: &[OsString],
Expand Down Expand Up @@ -130,7 +136,10 @@ mod test {

fn parse_arguments_(arguments: Vec<String>) -> CompilerArguments<ParsedArguments> {
let arguments = arguments.iter().map(OsString::from).collect::<Vec<_>>();
Clang.parse_arguments(&arguments, ".".as_ref())
Clang {
clangplusplus: false,
}
.parse_arguments(&arguments, ".".as_ref())
}

macro_rules! parses {
Expand Down
36 changes: 33 additions & 3 deletions src/compiler/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -970,8 +970,12 @@ nvcc
msvc-clang
#elif defined(_MSC_VER)
msvc
#elif defined(__clang__) && defined(__cplusplus)
clang++
#elif defined(__clang__)
clang
#elif defined(__GNUC__) && defined(__cplusplus)
g++
#elif defined(__GNUC__)
gcc
#elif defined(__DCC__)
Expand Down Expand Up @@ -1008,11 +1012,30 @@ diab
for line in stdout.lines() {
//TODO: do something smarter here.
match line {
"clang++" => {
debug!("Found clang++");
return Box::new(
CCompiler::new(
Clang {
clangplusplus: true,
},
executable,
&pool,
)
.map(|c| Box::new(c) as Box<dyn Compiler<T>>),
);
}
"clang" => {
debug!("Found clang");
return Box::new(
CCompiler::new(Clang, executable, &pool)
.map(|c| Box::new(c) as Box<dyn Compiler<T>>),
CCompiler::new(
Clang {
clangplusplus: false,
},
executable,
&pool,
)
.map(|c| Box::new(c) as Box<dyn Compiler<T>>),
);
}
"diab" => {
Expand All @@ -1022,10 +1045,17 @@ diab
.map(|c| Box::new(c) as Box<dyn Compiler<T>>),
);
}
"g++" => {
debug!("Found G++");
return Box::new(
CCompiler::new(GCC { gplusplus: true }, executable, &pool)
.map(|c| Box::new(c) as Box<dyn Compiler<T>>),
);
}
"gcc" => {
debug!("Found GCC");
return Box::new(
CCompiler::new(GCC, executable, &pool)
CCompiler::new(GCC { gplusplus: false }, executable, &pool)
.map(|c| Box::new(c) as Box<dyn Compiler<T>>),
);
}
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/diab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ impl CCompilerImpl for Diab {
fn kind(&self) -> CCompilerKind {
CCompilerKind::Diab
}
fn plusplus(&self) -> bool {
false
}
fn parse_arguments(
&self,
arguments: &[OsString],
Expand Down
9 changes: 7 additions & 2 deletions src/compiler/gcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,19 @@ use std::process;

use crate::errors::*;

/// A unit struct on which to implement `CCompilerImpl`.
/// A struct on which to implement `CCompilerImpl`.
#[derive(Clone, Debug)]
pub struct GCC;
pub struct GCC {
pub gplusplus: bool,
}

impl CCompilerImpl for GCC {
fn kind(&self) -> CCompilerKind {
CCompilerKind::GCC
}
fn plusplus(&self) -> bool {
self.gplusplus
}
fn parse_arguments(
&self,
arguments: &[OsString],
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/msvc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ impl CCompilerImpl for MSVC {
fn kind(&self) -> CCompilerKind {
CCompilerKind::MSVC
}
fn plusplus(&self) -> bool {
false
}
fn parse_arguments(
&self,
arguments: &[OsString],
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/nvcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ impl CCompilerImpl for NVCC {
fn kind(&self) -> CCompilerKind {
CCompilerKind::NVCC
}
fn plusplus(&self) -> bool {
false
}
fn parse_arguments(
&self,
arguments: &[OsString],
Expand Down

0 comments on commit feca129

Please sign in to comment.