From ebf6a318c162a0f12268bcb53ba7ac1f75e6f70d Mon Sep 17 00:00:00 2001 From: devtejasx Date: Sat, 1 Aug 2026 14:10:29 +0530 Subject: [PATCH] Fix a few clang-tidy findings in src/ (#1925) Three findings from the checks the project's own .clang-tidy already globs in, limited to changes that cannot alter behaviour: - readability-uppercase-literal-suffix: `0u` -> `0U`. - readability-simplify-boolean-expr: distribute the negation over the disjunction in RunInThread() and in IsTruthyFlagValue(), rather than negating the whole expression. - misc-use-anonymous-namespace: the file-local console reporter helpers IgnoreColorPrint() and FormatTime() move into an anonymous namespace instead of being marked static. This deliberately leaves out misc-const-correctness, which accounts for most of what clang-tidy reports here and is the part that is purely a matter of taste. Happy to add or drop any of the above. No functional change. Release and debug builds are clean and the test suite passes. --- src/benchmark_runner.cc | 12 ++++++------ src/commandlineflags.cc | 4 ++-- src/console_reporter.cc | 10 +++++++--- src/csv_reporter.cc | 2 +- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/benchmark_runner.cc b/src/benchmark_runner.cc index f6d37e0172..af77e1506b 100644 --- a/src/benchmark_runner.cc +++ b/src/benchmark_runner.cc @@ -105,7 +105,7 @@ BenchmarkReporter::Run CreateRunReport( report.repetition_index = repetition_index; report.repetitions = repeats; - if (report.skipped == 0u) { + if (report.skipped == 0U) { if (b.use_manual_time()) { report.real_accumulated_time = results.manual_time_used; } else { @@ -152,7 +152,7 @@ void RunInThread(const BenchmarkInstance* b, IterationCount iters, State st = b->Run(iters, thread_id, &timer, manager, perf_counters_measurement, profiler_manager_); - if (!(st.skipped() || st.iterations() >= st.max_iterations)) { + if (!st.skipped() && st.iterations() < st.max_iterations) { st.SkipWithError( "The benchmark didn't run, nor was it explicitly skipped. Please call " "'SkipWithXXX` in your benchmark as appropriate."); @@ -321,10 +321,10 @@ BenchmarkRunner::BenchmarkRunner( if (b.aggregation_report_mode() != internal::ARM_Unspecified) { run_results.display_report_aggregates_only = ((b.aggregation_report_mode() & - internal::ARM_DisplayReportAggregatesOnly) != 0u); + internal::ARM_DisplayReportAggregatesOnly) != 0U); run_results.file_report_aggregates_only = ((b.aggregation_report_mode() & - internal::ARM_FileReportAggregatesOnly) != 0u); + internal::ARM_FileReportAggregatesOnly) != 0U); BM_CHECK(FLAGS_benchmark_perf_counters.empty() || (perf_counters_measurement_ptr->num_counters() == 0)) << "Perf counters were requested but could not be set up."; @@ -399,7 +399,7 @@ bool BenchmarkRunner::ShouldReportIterationResults( // Determine if this run should be reported; // Either it has run for a sufficient amount of time // or because an error was reported. - return (i.results.skipped_ != 0u) || FLAGS_benchmark_dry_run || + return (i.results.skipped_ != 0U) || FLAGS_benchmark_dry_run || i.iters >= kMaxIterations || // Too many iterations already. i.seconds >= GetMinTimeToApply() || // The elapsed time is large enough. @@ -560,7 +560,7 @@ void BenchmarkRunner::DoOneRepetition() { if (reports_for_family != nullptr) { ++reports_for_family->num_runs_done; - if (report.skipped == 0u) { + if (report.skipped == 0U) { reports_for_family->Runs.push_back(report); } } diff --git a/src/commandlineflags.cc b/src/commandlineflags.cc index 99a240c122..80edfb4360 100644 --- a/src/commandlineflags.cc +++ b/src/commandlineflags.cc @@ -309,8 +309,8 @@ BENCHMARK_EXPORT bool IsTruthyFlagValue(const std::string& value) { if (value.size() == 1) { char v = value[0]; - return isalnum(v) && - !(v == '0' || v == 'f' || v == 'F' || v == 'n' || v == 'N'); + return isalnum(v) && v != '0' && v != 'f' && v != 'F' && v != 'n' && + v != 'N'; } if (!value.empty()) { std::string value_lower(value); diff --git a/src/console_reporter.cc b/src/console_reporter.cc index 84fe99dad1..bb397e9ca5 100644 --- a/src/console_reporter.cc +++ b/src/console_reporter.cc @@ -99,16 +99,18 @@ void ConsoleReporter::ReportRuns(const std::vector& reports) { } } +namespace { + PRINTF_FORMAT_STRING_FUNC(3, 4) -static void IgnoreColorPrint(std::ostream& out, LogColor /*unused*/, - const char* fmt, ...) { +void IgnoreColorPrint(std::ostream& out, LogColor /*unused*/, const char* fmt, + ...) { va_list args; va_start(args, fmt); out << FormatString(fmt, args); va_end(args); } -static std::string FormatTime(double time) { +std::string FormatTime(double time) { // For the time columns of the console printer 13 digits are reserved. One of // them is a space and max two of them are the time unit (e.g ns). That puts // us at 10 digits usable for the number. @@ -130,6 +132,8 @@ static std::string FormatTime(double time) { return FormatString("%10.0f", time); } +} // namespace + BENCHMARK_EXPORT void ConsoleReporter::PrintRunData(const Run& result) { typedef void(PrinterFn)(std::ostream&, LogColor, const char*, ...); diff --git a/src/csv_reporter.cc b/src/csv_reporter.cc index 3e21d11f0a..99716368da 100644 --- a/src/csv_reporter.cc +++ b/src/csv_reporter.cc @@ -112,7 +112,7 @@ BENCHMARK_EXPORT void CSVReporter::PrintRunData(const Run& run) { std::ostream& Out = GetOutputStream(); Out << CsvEscape(run.benchmark_name()) << ","; - if (run.skipped != 0u) { + if (run.skipped != 0U) { Out << std::string(elements.size() - 3, ','); Out << std::boolalpha << (internal::SkippedWithError == run.skipped) << ","; Out << CsvEscape(run.skip_message) << "\n";