Skip to content

[DRAFT] Modernization of coding styles through fixing Clippy warnings #1808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
4 changes: 2 additions & 2 deletions crates/intrinsic-test/src/arm/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::common::compile_c::CompilationCommandBuilder;
use crate::common::gen_c::compile_c_programs;

pub fn compile_c_arm(
intrinsics_name_list: &Vec<String>,
intrinsics_name_list: &[String],
compiler: &str,
target: &str,
cxx_toolchain_dir: Option<&str>,
Expand Down Expand Up @@ -56,7 +56,7 @@ pub fn compile_c_arm(
.clone()
.set_input_name(intrinsic_name)
.set_output_name(intrinsic_name)
.to_string()
.make_string()
})
.collect::<Vec<_>>();

Expand Down
13 changes: 8 additions & 5 deletions crates/intrinsic-test/src/arm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ impl SupportedArchitectureTest for ArmArchitectureTest {
intrinsics.dedup();

Box::new(Self {
intrinsics: intrinsics,
cli_options: cli_options,
intrinsics,
cli_options,
})
}

Expand All @@ -71,9 +71,12 @@ impl SupportedArchitectureTest for ArmArchitectureTest {

match compiler {
None => true,
Some(compiler) => {
compile_c_arm(&intrinsics_name_list, compiler, target, cxx_toolchain_dir)
}
Some(compiler) => compile_c_arm(
intrinsics_name_list.as_slice(),
compiler,
target,
cxx_toolchain_dir,
),
}
}

Expand Down
25 changes: 14 additions & 11 deletions crates/intrinsic-test/src/arm/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ impl IntrinsicTypeDefinition for ArmIntrinsicType {
(self.0.bit_len, self.0.simd_len, self.0.vec_len)
{
match (simd_len, vec_len) {
(None, None) => format!("{}{}{}_t", const_prefix, prefix, bit_len),
(Some(simd), None) => format!("{}{bit_len}x{simd}_t", prefix),
(Some(simd), Some(vec)) => format!("{}{bit_len}x{simd}x{vec}_t", prefix),
(None, None) => format!("{const_prefix}{prefix}{bit_len}_t"),
(Some(simd), None) => format!("{prefix}{bit_len}x{simd}_t"),
(Some(simd), Some(vec)) => format!("{prefix}{bit_len}x{simd}x{vec}_t"),
(None, Some(_)) => todo!("{:#?}", self), // Likely an invalid case
}
} else {
Expand All @@ -24,25 +24,28 @@ impl IntrinsicTypeDefinition for ArmIntrinsicType {

fn c_single_vector_type(&self) -> String {
if let (Some(bit_len), Some(simd_len)) = (self.0.bit_len, self.0.simd_len) {
let prefix = self.0.kind.c_prefix();
format!("{}{bit_len}x{simd_len}_t", prefix)
format!(
"{prefix}{bit_len}x{simd_len}_t",
prefix = self.0.kind.c_prefix()
)
} else {
unreachable!("Shouldn't be called on this type")
}
}

fn rust_type(&self) -> String {
let rust_prefix = self.0.kind.rust_prefix();
let c_prefix = self.0.kind.rust_prefix();
let c_prefix = self.0.kind.c_prefix();
if self.0.ptr_constant {
self.c_type()
} else if let (Some(bit_len), simd_len, vec_len) =
(self.0.bit_len, self.0.simd_len, self.0.vec_len)
{
// TODO: investigation (c_prefix is created okay?)
match (simd_len, vec_len) {
(None, None) => format!("{}{bit_len}", rust_prefix),
(Some(simd), None) => format!("{}{bit_len}x{simd}_t", c_prefix),
(Some(simd), Some(vec)) => format!("{}{bit_len}x{simd}x{vec}_t", c_prefix),
(None, None) => format!("{rust_prefix}{bit_len}"),
(Some(simd), None) => format!("{c_prefix}{bit_len}x{simd}_t"),
(Some(simd), Some(vec)) => format!("{c_prefix}{bit_len}x{simd}x{vec}_t"),
(None, Some(_)) => todo!("{:#?}", self), // Likely an invalid case
}
} else {
Expand Down Expand Up @@ -128,11 +131,11 @@ impl IntrinsicTypeDefinition for ArmIntrinsicType {
};
let s = s.trim_end();
let temp_return = ArmIntrinsicType::from_c(s, target);
temp_return.and_then(|mut op| {
temp_return.map(|mut op| {
let edited = op.as_mut();
edited.0.ptr = true;
edited.0.ptr_constant = constant;
Ok(op)
op
})
} else {
// [const ]TYPE[{bitlen}[x{simdlen}[x{vec_len}]]][_t]
Expand Down
94 changes: 44 additions & 50 deletions crates/intrinsic-test/src/common/argument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ where
}

pub fn has_constraint(&self) -> bool {
!self.constraint.is_some()
// TODO: investigation (is_none is okay?)
self.constraint.is_some()
}

pub fn type_and_name_from_c(arg: &str) -> (&str, &str) {
Expand Down Expand Up @@ -127,15 +128,14 @@ where
/// e.g `const int32x2_t a_vals = {0x3effffff, 0x3effffff, 0x3f7fffff}`, if loads=2.
pub fn gen_arglists_c(&self, indentation: Indentation, loads: u32) -> String {
self.iter()
.filter_map(|arg| {
(!arg.has_constraint()).then(|| {
format!(
"{indentation}const {ty} {name}_vals[] = {values};",
ty = arg.ty.c_scalar_type(),
name = arg.name,
values = arg.ty.populate_random(indentation, loads, &Language::C)
)
})
.filter(|&arg| (!arg.has_constraint()))
.map(|arg| {
format!(
"{indentation}const {ty} {name}_vals[] = {values};",
ty = arg.ty.c_scalar_type(),
name = arg.name,
values = arg.ty.populate_random(indentation, loads, &Language::C)
)
})
.collect::<Vec<_>>()
.join("\n")
Expand All @@ -145,17 +145,16 @@ where
/// values can be loaded as a sliding window, e.g `const A_VALS: [u32; 20] = [...];`
pub fn gen_arglists_rust(&self, indentation: Indentation, loads: u32) -> String {
self.iter()
.filter_map(|arg| {
(!arg.has_constraint()).then(|| {
format!(
"{indentation}{bind} {name}: [{ty}; {load_size}] = {values};",
bind = arg.rust_vals_array_binding(),
name = arg.rust_vals_array_name(),
ty = arg.ty.rust_scalar_type(),
load_size = arg.ty.num_lanes() * arg.ty.num_vectors() + loads - 1,
values = arg.ty.populate_random(indentation, loads, &Language::Rust)
)
})
.filter(|&arg| (!arg.has_constraint()))
.map(|arg| {
format!(
"{indentation}{bind} {name}: [{ty}; {load_size}] = {values};",
bind = arg.rust_vals_array_binding(),
name = arg.rust_vals_array_name(),
ty = arg.ty.rust_scalar_type(),
load_size = arg.ty.num_lanes() * arg.ty.num_vectors() + loads - 1,
values = arg.ty.populate_random(indentation, loads, &Language::Rust)
)
})
.collect::<Vec<_>>()
.join("\n")
Expand All @@ -168,22 +167,18 @@ where
/// ARM-specific
pub fn load_values_c(&self, indentation: Indentation) -> String {
self.iter()
.filter_map(|arg| {
// The ACLE doesn't support 64-bit polynomial loads on Armv7
// This and the cast are a workaround for this

(!arg.has_constraint()).then(|| {
format!(
"{indentation}{ty} {name} = cast<{ty}>({load}(&{name}_vals[i]));\n",
ty = arg.to_c_type(),
name = arg.name,
load = if arg.is_simd() {
arg.ty.get_load_function(Language::C)
} else {
"*".to_string()
}
)
})
.filter(|&arg| (!arg.has_constraint()))
.map(|arg| {
format!(
"{indentation}{ty} {name} = cast<{ty}>({load}(&{name}_vals[i]));\n",
ty = arg.to_c_type(),
name = arg.name,
load = if arg.is_simd() {
arg.ty.get_load_function(Language::C)
} else {
"*".to_string()
}
)
})
.collect()
}
Expand All @@ -193,19 +188,18 @@ where
/// e.g `let a = vld1_u8(A_VALS.as_ptr().offset(i));`
pub fn load_values_rust(&self, indentation: Indentation) -> String {
self.iter()
.filter_map(|arg| {
(!arg.has_constraint()).then(|| {
format!(
"{indentation}let {name} = {load}({vals_name}.as_ptr().offset(i));\n",
name = arg.name,
vals_name = arg.rust_vals_array_name(),
load = if arg.is_simd() {
arg.ty.get_load_function(Language::Rust)
} else {
"*".to_string()
},
)
})
.filter(|&arg| (!arg.has_constraint()))
.map(|arg| {
format!(
"{indentation}let {name} = {load}({vals_name}.as_ptr().offset(i));\n",
name = arg.name,
vals_name = arg.rust_vals_array_name(),
load = if arg.is_simd() {
arg.ty.get_load_function(Language::Rust)
} else {
"*".to_string()
},
)
})
.collect()
}
Expand Down
16 changes: 8 additions & 8 deletions crates/intrinsic-test/src/common/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ impl ProcessedCli {
};

Self {
toolchain: toolchain,
cpp_compiler: cpp_compiler,
c_runner: c_runner,
target: target,
linker: linker,
cxx_toolchain_dir: cxx_toolchain_dir,
skip: skip,
filename: filename,
toolchain,
cpp_compiler,
c_runner,
target,
linker,
cxx_toolchain_dir,
skip,
filename,
}
}
}
4 changes: 2 additions & 2 deletions crates/intrinsic-test/src/common/compile_c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ impl CompilationCommandBuilder {
}

impl CompilationCommandBuilder {
pub fn to_string(self) -> String {
pub fn make_string(self) -> String {
let arch_flags = self.arch_flags.join("+");
let flags = std::env::var("CPPFLAGS").unwrap_or("".into());
let project_root = self.project_root.unwrap_or(String::new());
let project_root = self.project_root.unwrap_or_default();
let project_root_str = project_root.as_str();
let mut output = self.output.clone();
if self.linker.is_some() {
Expand Down
2 changes: 1 addition & 1 deletion crates/intrinsic-test/src/common/gen_c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ int main(int argc, char **argv) {{
.map(|header| format!("#include <{header}>"))
.collect::<Vec<_>>()
.join("\n"),
arch_specific_definitions = arch_specific_definitions.into_iter().join("\n"),
arch_specific_definitions = arch_specific_definitions.join("\n"),
)
}

Expand Down
2 changes: 1 addition & 1 deletion crates/intrinsic-test/src/common/gen_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ pub fn setup_rust_file_paths(identifiers: &Vec<String>) -> BTreeMap<&String, Str
identifiers
.par_iter()
.map(|identifier| {
let rust_dir = format!(r#"rust_programs/{}"#, identifier);
let rust_dir = format!(r#"rust_programs/{identifier}"#);
let _ = std::fs::create_dir_all(&rust_dir);
let rust_filename = format!(r#"{rust_dir}/main.rs"#);

Expand Down
6 changes: 3 additions & 3 deletions crates/intrinsic-test/src/common/intrinsic_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ impl IntrinsicType {
}

pub fn num_lanes(&self) -> u32 {
if let Some(sl) = self.simd_len { sl } else { 1 }
self.simd_len.unwrap_or(1)
}

pub fn num_vectors(&self) -> u32 {
if let Some(vl) = self.vec_len { vl } else { 1 }
self.vec_len.unwrap_or(1)
}

pub fn is_simd(&self) -> bool {
Expand Down Expand Up @@ -266,7 +266,7 @@ impl IntrinsicType {

pub fn as_call_param_c(&self, name: &String) -> String {
if self.ptr {
format!("&{}", name)
format!("&{name}")
} else {
name.clone()
}
Expand Down
12 changes: 5 additions & 7 deletions crates/intrinsic-test/src/common/write_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::fs::File;
use std::io::Write;

pub fn write_file(filename: &String, code: String) {
let mut file = File::create(&filename).unwrap();
let mut file = File::create(filename).unwrap();
file.write_all(code.into_bytes().as_slice()).unwrap();
}

Expand All @@ -34,9 +34,8 @@ pub fn write_c_testfiles<T: IntrinsicTypeDefinition + Sized>(
notice,
arch_specific_definitions,
);
match filename_mapping.get(&i.name()) {
Some(filename) => write_file(filename, c_code),
None => {}
if let Some(filename) = filename_mapping.get(&i.name()) {
write_file(filename, c_code)
};
});

Expand All @@ -58,9 +57,8 @@ pub fn write_rust_testfiles<T: IntrinsicTypeDefinition>(

intrinsics.iter().for_each(|&i| {
let rust_code = create_rust_test_program(i, rust_target, notice, definitions, cfg);
match filename_mapping.get(&i.name()) {
Some(filename) => write_file(filename, rust_code),
None => {}
if let Some(filename) = filename_mapping.get(&i.name()) {
write_file(filename, rust_code)
}
});

Expand Down
Loading