Skip to content

Commit 6e6e43d

Browse files
authored
perf: optimize use of ClangParams struct (#231)
This clones a reference (pointer) to the same `ClangParams` instance instead of cloning the instance data (when processing each source file). Some param types updated to reflect the actual life cycle of the `ClangParams` instance. Another param type change relies on internal mutability when passing around references to `FileObj` instances in an array. Added auto-trait `Debug` to `ClangVersions` struct, in case debug printing is ever needed. Removes a duplicate condition where ranges of changed lines are an empty list if `lines-changed-only` is `Off`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Optimized memory handling in the analysis pipeline for improved efficiency. * Refined line-filter logic for more consistent processing. * **Tests** * Updated test variable naming for clarity. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent fde6ce5 commit 6e6e43d

File tree

3 files changed

+22
-23
lines changed

3 files changed

+22
-23
lines changed

cpp-linter/src/clang_tools/clang_tidy.rs

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use serde::Deserialize;
1717
// project-specific modules/crates
1818
use super::MakeSuggestions;
1919
use crate::{
20-
cli::{ClangParams, LinesChangedOnly},
20+
cli::ClangParams,
2121
common_fs::{FileObj, normalize_path},
2222
};
2323

@@ -263,19 +263,17 @@ pub fn run_clang_tidy(
263263
cmd.args(["--extra-arg", format!("\"{}\"", arg).as_str()]);
264264
}
265265
let file_name = file.name.to_string_lossy().to_string();
266-
if clang_params.lines_changed_only != LinesChangedOnly::Off {
267-
let ranges = file.get_ranges(&clang_params.lines_changed_only);
268-
if !ranges.is_empty() {
269-
let filter = format!(
270-
"[{{\"name\":{:?},\"lines\":{:?}}}]",
271-
&file_name.replace('/', if OS == "windows" { "\\" } else { "/" }),
272-
ranges
273-
.iter()
274-
.map(|r| [r.start(), r.end()])
275-
.collect::<Vec<_>>()
276-
);
277-
cmd.args(["--line-filter", filter.as_str()]);
278-
}
266+
let ranges = file.get_ranges(&clang_params.lines_changed_only);
267+
if !ranges.is_empty() {
268+
let filter = format!(
269+
"[{{\"name\":{:?},\"lines\":{:?}}}]",
270+
&file_name.replace('/', if OS == "windows" { "\\" } else { "/" }),
271+
ranges
272+
.iter()
273+
.map(|r| [r.start(), r.end()])
274+
.collect::<Vec<_>>()
275+
);
276+
cmd.args(["--line-filter", filter.as_str()]);
279277
}
280278
let original_content = if !clang_params.tidy_review {
281279
None
@@ -429,7 +427,7 @@ mod test {
429427
)
430428
.unwrap();
431429
let file = FileObj::new(PathBuf::from("tests/demo/demo.cpp"));
432-
let arc_ref = Arc::new(Mutex::new(file));
430+
let arc_file = Arc::new(Mutex::new(file));
433431
let extra_args = vec!["-std=c++17".to_string(), "-Wall".to_string()];
434432
let clang_params = ClangParams {
435433
style: "".to_string(),
@@ -445,7 +443,7 @@ mod test {
445443
clang_tidy_command: Some(exe_path),
446444
clang_format_command: None,
447445
};
448-
let mut file_lock = arc_ref.lock().unwrap();
446+
let mut file_lock = arc_file.lock().unwrap();
449447
let logs = run_clang_tidy(&mut file_lock, &clang_params)
450448
.unwrap()
451449
.into_iter()

cpp-linter/src/clang_tools/mod.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ fn analyze_single_file(
185185
}
186186

187187
/// A struct to contain the version numbers of the clang-tools used
188-
#[derive(Default)]
188+
#[derive(Debug, Default)]
189189
pub struct ClangVersions {
190190
/// The clang-format version used.
191191
pub format_version: Option<String>,
@@ -199,9 +199,9 @@ pub struct ClangVersions {
199199
/// If `tidy_checks` is `"-*"` then clang-tidy is not executed.
200200
/// If `style` is a blank string (`""`), then clang-format is not executed.
201201
pub async fn capture_clang_tools_output(
202-
files: &mut Vec<Arc<Mutex<FileObj>>>,
202+
files: &[Arc<Mutex<FileObj>>],
203203
version: &RequestedVersion,
204-
clang_params: &mut ClangParams,
204+
mut clang_params: ClangParams,
205205
rest_api_client: &impl RestApiClient,
206206
) -> Result<ClangVersions> {
207207
let mut clang_versions = ClangVersions::default();
@@ -240,10 +240,11 @@ pub async fn capture_clang_tools_output(
240240
};
241241

242242
let mut executors = JoinSet::new();
243+
let arc_params = Arc::new(clang_params);
243244
// iterate over the discovered files and run the clang tools
244245
for file in files {
245-
let arc_params = Arc::new(clang_params.clone());
246-
let arc_file = Arc::clone(file);
246+
let arc_file = file.clone();
247+
let arc_params = arc_params.clone();
247248
executors.spawn(async move { analyze_single_file(arc_file, arc_params) });
248249
}
249250

cpp-linter/src/run.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
138138
clang_params.tidy_review &= is_pr;
139139
let user_inputs = FeedbackInput::from(&cli);
140140
let clang_versions = capture_clang_tools_output(
141-
&mut arc_files,
141+
&arc_files,
142142
&cli.general_options.version,
143-
&mut clang_params,
143+
clang_params,
144144
&rest_api_client,
145145
)
146146
.await?;

0 commit comments

Comments
 (0)