From 67aa3a3216e8dcf63b30f1978dbb4cca149b723b Mon Sep 17 00:00:00 2001 From: Romaric Jodin Date: Wed, 18 Dec 2024 17:15:37 +0100 Subject: [PATCH] refactor for loops (#66) --- .github/workflows/presubmit.yml | 1 + src/context.rs | 14 ++--- src/ninja_target/common.rs | 50 +++++++----------- src/parser.rs | 15 +++--- src/project/angle.rs | 8 +-- src/project/clspv.rs | 12 ++--- src/project/llvm_project.rs | 9 +--- src/soong_module.rs | 10 ++-- src/soong_package.rs | 94 +++++++++++++-------------------- src/utils.rs | 15 ++++-- 10 files changed, 98 insertions(+), 130 deletions(-) diff --git a/.github/workflows/presubmit.yml b/.github/workflows/presubmit.yml index bfa93dc..07f27ea 100644 --- a/.github/workflows/presubmit.yml +++ b/.github/workflows/presubmit.yml @@ -50,6 +50,7 @@ jobs: N2S_NDK_PATH: ${{ env.android }} run: | set -x + mkdir -p "${{ env.android }}" PATH="${{ env.android }}/depot_tools:${PATH}" cargo run --release -- --skip-gen-ninja --angle-path "${{ env.android }}/external/angle" --aosp-path "${{ env.android }}" - name: Check generated files shell: bash diff --git a/src/context.rs b/src/context.rs index 56bc10b..05ae435 100644 --- a/src/context.rs +++ b/src/context.rs @@ -26,18 +26,18 @@ pub struct Context { impl Context { fn help(&self, projects: &Vec<&mut dyn Project>) -> String { - let mut projects_help = String::new(); - for project in projects { - projects_help += " "; - projects_help += project.get_id().str(); - projects_help += "\n"; - } + let projects_help = projects + .iter() + .map(|project| project.get_id().str()) + .collect::>() + .join("\n "); format!( " USAGE: {0} [OPTIONS] [PROJECTS] PROJECTS: -{projects_help} + {projects_help} + OPTIONS: {ANGLE_PATH} Path to angle source repository (required only for the `angle` project) {AOSP_PATH} Path to Android tree (required for most project) diff --git a/src/ninja_target/common.rs b/src/ninja_target/common.rs index 3631284..73e1196 100644 --- a/src/ninja_target/common.rs +++ b/src/ninja_target/common.rs @@ -28,27 +28,21 @@ pub fn get_link_libraries(libs: &str) -> Result<(Vec, Vec), St Ok((static_libraries, shared_libraries)) } -pub fn get_defines(defs: &str) -> Vec { - let mut defines = Vec::new(); - for define in defs.split("-D") { - if define.is_empty() { - continue; - } - defines.push(define.trim().replace("\\(", "(").replace("\\)", ")")); - } +pub fn get_defines(defines: &str) -> Vec { defines + .split("-D") + .filter(|define| !define.is_empty()) + .map(|define| define.trim().replace("\\(", "(").replace("\\)", ")")) + .collect() } -pub fn get_includes(incs: &str, build_path: &Path) -> Vec { - let mut includes = Vec::new(); - for inc in incs.split(" ") { - let include = inc.strip_prefix("-I").unwrap_or(inc); - if include.is_empty() || include == "isystem" { - continue; - } - includes.push(canonicalize_path(include, build_path)); - } +pub fn get_includes(includes: &str, build_path: &Path) -> Vec { includes + .split(" ") + .map(|include| include.strip_prefix("-I").unwrap_or(include)) + .filter(|include| !(include.is_empty() || *include == "isystem")) + .map(|include| canonicalize_path(include, build_path)) + .collect() } pub fn get_link_flags(flags: &str) -> (Option, Vec) { @@ -64,20 +58,16 @@ pub fn get_link_flags(flags: &str) -> (Option, Vec) { } pub fn get_cflags(flags: &str) -> Vec { - let mut cflags = Vec::new(); - for flag in flags.split(" ") { - if flag.is_empty() { - continue; - } - cflags.push(flag.to_string()); - } - cflags + flags + .split(" ") + .filter(|flag| !flag.is_empty()) + .map(|flag| flag.to_string()) + .collect() } -pub fn get_sources(ins: &Vec, build_path: &Path) -> Vec { - let mut inputs = Vec::new(); - for input in ins { - inputs.push(canonicalize_path(input, build_path)); - } +pub fn get_sources(inputs: &Vec, build_path: &Path) -> Vec { inputs + .into_iter() + .map(|input| canonicalize_path(input, build_path)) + .collect() } diff --git a/src/parser.rs b/src/parser.rs index 8e0de58..e52a9d6 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -11,10 +11,10 @@ fn parse_output_section(section: &str) -> Result<(Vec, Vec), S if split.clone().count() < 1 { return error!("parse_output_section failed: '{section}'"); } - let mut outputs = Vec::new(); - for output in split.nth(0).unwrap().trim().split(" ") { - outputs.push(PathBuf::from(output.trim())); - } + let split_outputs = split.nth(0).unwrap().trim().split(" "); + let outputs = split_outputs + .map(|output| PathBuf::from(output.trim())) + .collect(); let mut implicit_outputs = Vec::new(); if let Some(implicit_outs) = split.next() { for implicit_output in implicit_outs.trim().split(" ") { @@ -30,10 +30,9 @@ fn parse_input_and_rule_section(section: &str) -> Result<(String, Vec), return error!("parse_input_and_rule_section failed: '{section}'"); } let rule = String::from(split.nth(0).unwrap()); - let mut inputs = Vec::new(); - for input in split { - inputs.push(PathBuf::from(input.trim())); - } + let inputs = split + .map(|input| PathBuf::from(input.trim())) + .collect::>(); Ok((rule, inputs)) } diff --git a/src/project/angle.rs b/src/project/angle.rs index 0d53b31..2f68393 100644 --- a/src/project/angle.rs +++ b/src/project/angle.rs @@ -100,10 +100,10 @@ impl Project for Angle { "SPDX-license-identifier-Apache-2.0", "LICENSE", ); - let mut targets_to_generate = Vec::new(); - for target in TARGETS { - targets_to_generate.push(PathBuf::from(target)); - } + let targets_to_generate = TARGETS + .into_iter() + .map(|target| PathBuf::from(target)) + .collect(); package.generate(targets_to_generate, targets, self)?; Ok(package) diff --git a/src/project/clspv.rs b/src/project/clspv.rs index 2edd595..17e79f9 100644 --- a/src/project/clspv.rs +++ b/src/project/clspv.rs @@ -103,12 +103,12 @@ impl Project for Clspv { let mut deps: GenDepsMap = HashMap::new(); match project { ProjectId::SpirvHeaders => { - let mut files = Vec::new(); - for dep in &self.gen_deps { - if dep.starts_with(&self.spirv_headers_path) { - files.push(dep.clone()); - } - } + let files = self + .gen_deps + .iter() + .filter(|dep| dep.starts_with(&self.spirv_headers_path)) + .map(|dep| dep.clone()) + .collect(); deps.insert(GenDeps::SpirvHeaders, files); } ProjectId::LlvmProject => { diff --git a/src/project/llvm_project.rs b/src/project/llvm_project.rs index 61e22b3..84d6d27 100644 --- a/src/project/llvm_project.rs +++ b/src/project/llvm_project.rs @@ -80,9 +80,7 @@ impl Project for LlvmProject { "tools/clang/include/clang/Basic/Version.inc", "tools/clang/include/clang/Config/config.h", ]; - for header in missing_gen_deps { - gen_deps.insert(PathBuf::from(header)); - } + gen_deps.extend(missing_gen_deps.iter().map(|dep| PathBuf::from(dep))); let mut gen_deps_folders: HashSet = HashSet::new(); for gen_dep in &gen_deps { @@ -201,10 +199,7 @@ impl Project for LlvmProject { "-DBLAKE3_NO_SSE2", ]); } - cflags.iter().fold(Vec::new(), |mut vec, flag| { - vec.push(flag.to_string()); - vec - }) + cflags.iter().map(|flag| flag.to_string()).collect() } fn get_include(&self, include: &Path) -> PathBuf { diff --git a/src/soong_module.rs b/src/soong_module.rs index b68be70..d235590 100644 --- a/src/soong_module.rs +++ b/src/soong_module.rs @@ -20,12 +20,8 @@ impl CcLibraryHeaders { } } -fn print_indent(level: u8) -> String { - let mut indent = String::new(); - for _ in 0..level { - indent += " "; - } - indent +fn print_indent(level: usize) -> String { + " ".repeat(level) } #[derive(Debug)] @@ -54,7 +50,7 @@ impl SoongNamedProp { } } - fn print(self, indent_level: u8) -> String { + fn print(self, indent_level: usize) -> String { let mut output = String::new(); output += &format!("{0}{1}: ", print_indent(indent_level), self.name); output += &(match self.prop { diff --git a/src/soong_package.rs b/src/soong_package.rs index 0fc5b22..afc0631 100644 --- a/src/soong_package.rs +++ b/src/soong_package.rs @@ -8,43 +8,27 @@ use crate::project::Project; use crate::soong_module::*; use crate::utils::*; -fn update_cflags_with_defines( - defines: Vec, - project: &dyn Project, - cflags: &mut HashSet, -) { - for define in defines { - if project.ignore_define(&define) { - continue; - } - cflags.insert("-D".to_string() + &project.get_define(&define)); - } +fn update_cflags_with_defines(defines: Vec, project: &dyn Project) -> Vec { + defines + .iter() + .filter(|def| !project.ignore_define(def)) + .map(|def| "-D".to_string() + &project.get_define(def)) + .collect() } -fn update_cflags(cflags: Vec, project: &dyn Project, all_cflags: &mut HashSet) { - for cflag in cflags { - if project.ignore_cflag(&cflag) { - continue; - } - all_cflags.insert(cflag); - } +fn update_cflags(cflags: Vec, project: &dyn Project) -> Vec { + cflags + .into_iter() + .filter(|cflag| !project.ignore_cflag(cflag)) + .collect() } -fn update_includes( - incs: Vec, - project: &dyn Project, - includes: &mut HashSet, - src_path: &Path, -) { - for include in incs { - if project.ignore_include(&include) { - continue; - } - includes.insert(path_to_string(strip_prefix( - project.get_include(&include), - src_path, - ))); - } +fn update_includes(includes: Vec, project: &dyn Project, src_path: &Path) -> Vec { + includes + .into_iter() + .filter(|include| !project.ignore_include(include)) + .map(|inc| path_to_string(strip_prefix(project.get_include(&inc), src_path))) + .collect() } #[derive(Debug)] @@ -184,32 +168,28 @@ impl<'a> SoongPackage<'a> { static_libs.extend(static_libraries); shared_libs.extend(shared_libraries); - update_includes( + includes.extend(update_includes( target.get_includes(self.build_path), project, - &mut includes, self.src_path, - ); - update_cflags_with_defines(target.get_defines(), project, &mut cflags); - update_cflags(target.get_cflags(), project, &mut cflags); + )); + cflags.extend(update_cflags_with_defines(target.get_defines(), project)); + cflags.extend(update_cflags(target.get_cflags(), project)); } - update_includes( + includes.extend(update_includes( target.get_includes(self.build_path), project, - &mut includes, self.src_path, - ); - update_cflags_with_defines(target.get_defines(), project, &mut cflags); - update_cflags(target.get_cflags(), project, &mut cflags); + )); + cflags.extend(update_cflags_with_defines(target.get_defines(), project)); + cflags.extend(update_cflags(target.get_cflags(), project)); let (version_script, link_flags) = target.get_link_flags(); - let link_flags = link_flags.into_iter().fold(Vec::new(), |mut vec, flag| { - if !project.ignore_link_flag(&flag) { - vec.push(flag) - } - vec - }); + let link_flags = link_flags + .into_iter() + .filter(|flag| !project.ignore_link_flag(flag)) + .collect::>(); let (static_libraries, shared_libraries) = target.get_link_libraries()?; static_libs.extend(static_libraries); shared_libs.extend(shared_libraries); @@ -376,20 +356,18 @@ impl<'a> SoongPackage<'a> { } if let Some((rsp_file, rsp_content)) = rule_cmd.1 { let rsp = "$(genDir)/".to_string() + &rsp_file; - let mut rsp_files = Vec::new(); - for file in rsp_content.split(" ") { - if file.is_empty() { - continue; - } - rsp_files.push( + let rsp_files = rsp_content + .split(" ") + .filter(|file| !file.is_empty()) + .map(|file| { String::from("$(location ") + &path_to_string(strip_prefix( canonicalize_path(file, self.build_path), self.src_path, )) - + ")", - ); - } + + ")" + }) + .collect::>(); cmd = "echo \\\"".to_string() + &rsp_files.join(" ") + "\\\" > " + &rsp + " && " + &cmd; cmd = cmd.replace("${rspfile}", &rsp); } diff --git a/src/utils.rs b/src/utils.rs index 1ae492a..4190290 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -106,8 +106,14 @@ pub fn canonicalize_path>(path: P, build_path: &Path) -> PathBuf } else { build_path .join(&path_buf) - .canonicalize() - .unwrap_or(path_buf) + .components() + .fold(PathBuf::new(), |path, component| { + if component == std::path::Component::ParentDir { + path.parent().unwrap().to_path_buf() + } else { + path.join(component) + } + }) } } @@ -155,7 +161,10 @@ pub fn split_path(path: &Path, delimiter: &str) -> Option<(PathBuf, PathBuf)> { } pub fn dep_name>(from: &Path, prefix: P, path: &str, build_path: &Path) -> String { - path_to_id(Path::new(path).join(strip_prefix(canonicalize_path(from, build_path), prefix))) + path_to_id(Path::new(path).join(strip_prefix( + canonicalize_path(from, build_path), + canonicalize_path(prefix, build_path), + ))) } pub fn copy_file(from: &Path, to: &Path) -> Result<(), String> {