Skip to content

Coding style modernization to deal with Clippy warnings #1812

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

Merged
merged 6 commits into from
May 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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
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
4 changes: 2 additions & 2 deletions crates/intrinsic-test/src/arm/json_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ struct JsonIntrinsic {

pub fn get_neon_intrinsics(
filename: &Path,
target: &String,
target: &str,
) -> Result<Vec<Intrinsic<ArmIntrinsicType>>, Box<dyn std::error::Error>> {
let file = std::fs::File::open(filename)?;
let reader = std::io::BufReader::new(file);
Expand All @@ -75,7 +75,7 @@ pub fn get_neon_intrinsics(

fn json_to_intrinsic(
mut intr: JsonIntrinsic,
target: &String,
target: &str,
) -> Result<Intrinsic<ArmIntrinsicType>, Box<dyn std::error::Error>> {
let name = intr.name.replace(['[', ']'], "");

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
24 changes: 13 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,8 +24,10 @@ 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")
}
Expand All @@ -40,9 +42,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!("{}{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 @@ -119,7 +121,7 @@ impl IntrinsicTypeDefinition for ArmIntrinsicType {
}
}

fn from_c(s: &str, target: &String) -> Result<Box<Self>, String> {
fn from_c(s: &str, target: &str) -> Result<Box<Self>, String> {
const CONST_STR: &str = "const";
if let Some(s) = s.strip_suffix('*') {
let (s, constant) = match s.trim().strip_suffix(CONST_STR) {
Expand All @@ -128,11 +130,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
95 changes: 44 additions & 51 deletions crates/intrinsic-test/src/common/argument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ where
}

pub fn has_constraint(&self) -> bool {
!self.constraint.is_some()
self.constraint.is_none()
}

pub fn type_and_name_from_c(arg: &str) -> (&str, &str) {
Expand Down Expand Up @@ -65,7 +65,7 @@ where
pub fn from_c(
pos: usize,
arg: &str,
target: &String,
target: &str,
constraint: Option<Constraint>,
) -> Argument<T> {
let (ty, var_name) = Self::type_and_name_from_c(arg);
Expand Down Expand Up @@ -127,15 +127,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 +144,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 +166,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 +187,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
4 changes: 2 additions & 2 deletions crates/intrinsic-test/src/common/gen_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,9 @@ 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!("rust_programs/{identifier}");
let _ = std::fs::create_dir_all(&rust_dir);
let rust_filename = format!(r#"{rust_dir}/main.rs"#);
let rust_filename = format!("{rust_dir}/main.rs");

(identifier, rust_filename)
})
Expand Down
8 changes: 4 additions & 4 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 All @@ -282,7 +282,7 @@ pub trait IntrinsicTypeDefinition: Deref<Target = IntrinsicType> {
fn get_lane_function(&self) -> String;

/// can be implemented in an `impl` block
fn from_c(_s: &str, _target: &String) -> Result<Box<Self>, String>;
fn from_c(_s: &str, _target: &str) -> Result<Box<Self>, String>;

/// Gets a string containing the typename for this type in C format.
/// can be directly defined in `impl` blocks
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